Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(subscriber): Reduce default retention to six seconds #504

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

grahamking
Copy link
Contributor

tokio-console's default retain_for is six seconds. These two values should match, no point send tokio-console information it's not going to use.

Fixes the display issue where on initial connect tokio-console will show hundreds of tasks, and then immediately stop displaying them.

Reduces default memory usage.

`tokio-console`'s default `retain_for` is six seconds. These two values
should match, no point send tokio-console information it's not going to
use.

Fixes the display issue where on initial connect tokio-console
will show hundreds of tasks, and then immediately stop displaying them.

Reduces default memory usage.
@grahamking grahamking requested a review from a team as a code owner December 15, 2023 22:31
@hawkw
Copy link
Member

hawkw commented Dec 16, 2023

The reason that the retention periods for console-subscriber and for the tokio-console CLI are different is because they configure very different things. The console-subscriber retention period configures how long the instrumented application stores data for tasks that have terminated. Storing this data while no tokio-console CLI client allows the console to display data such as the parent tasks in which currently active tasks have spawned, even if the parent task has terminated, and to display other historical data in the console. Therefore, we use a fairly long retention period. The CLI retention period, on the other hand, is used to configure how long recently-terminated tasks are displayed in the UI.

I'm definitely open to changes to these default values, as they were picked pretty arbitrarily, but it's important to consider that they're controlling fairly different things.

@grahamking
Copy link
Contributor Author

I didn't realise that thanks.

The trouble I hit, and I think many people hit, is that the one hour retention looks an awful lot like a memory leak. Memory grows (often quite rapidly) for the first hour. It also makes the message too big for tokio-console to decode, so after a few minutes that stopped working for me (hopefully #503 will help).

Would it be possible to only keep the objects that are a parent? I haven't looked yet, but if you think it can be done I could try working on that instead.

@hawkw
Copy link
Member

hawkw commented Jan 22, 2024

Would it be possible to only keep the objects that are a parent? I haven't looked yet, but if you think it can be done I could try working on that instead.

Hmm, we could look into something like that. It's somewhat complex, because we don't know if a span will be the parent of another span when the span is created; it only becomes a parent when another span is created inside it. However, we could track whether a closed span is the parent of a currently open span, and only retain spans if they are currently live or the parent of a currently live span. That would allow us to retain significantly less data. If you're still interested to work on that, I would be happy to merge a PR making such a change, and I can help answer questions about how to implement it.

Another option for reducing memory use, which might be a bigger project, is to change the console-subscriber so that when no client is connected, historical data is persisted to disk rather than stored in memory. When a client connects, we re-load the data stored on disk and send it to that client. We would still want to retain data on disk for a fixed time window, so that we don't use an unbounded amount of disk space. There is currently some rudimentary support for recording a log of console events to a file, but we don't currently use this recording to play back data for new clients.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants