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.

Sign up for GitHub

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

Jump to bottom

tokio-console cannot show the pause status correctly #551

Closed
Rustin170506 opened this issue May 9, 2024 · 5 comments · Fixed by #582
Closed

tokio-console cannot show the pause status correctly #551

Rustin170506 opened this issue May 9, 2024 · 5 comments · Fixed by #582
Assignees
@Rustin170506
Labels
C-console Crate: console. C-subscriber Crate: console-subscriber. S-bug Severity: bug

Comments

@Rustin170506
Copy link
Collaborator

What crate(s) in this repo are involved in the problem?

tokio-console

What is the issue?

There are two Tokio Console instances connected to the same stream. If you pause the stream in one instance and the status cannot be displayed correctly in the other instance.

How can the bug be reproduced?

All steps execute under the git repo:

  1. Start the example app: cargo run --example app
  2. Start one tokio console: cargo run
  3. Start another tokio console: cargo run
  4. Hit space in one of the instances
image As you can see, all of them are paused, but only one of them showed `PAUSED`.

Logs, error output, etc

No response

Versions

commit 3193fde77adf1473c4124adfa6ef8234659589c9 (origin/main, origin/HEAD)
Author: 二手掉包工程师 <[email protected]>
Date:   Mon Apr 22 21:35:21 2024 +0800

    chore: run clippy against all targets (#546)
    
    This would help us to check if there are any
    clippy warning in the tests.

Possible solution

Perhaps we should include a new API to allow clients to check whether the stream has been paused.

Additional context

I believe the reason for this problem is that we only store the pause status on the client side. Therefore, there is no way to notify other instances to know the stream is paused.

Would you like to work on fixing this bug?

yes
@Rustin170506 Rustin170506 added S-bug Severity: bug C-console Crate: console. C-subscriber Crate: console-subscriber. C-api Crate: console-api labels May 9, 2024
@Rustin170506
Copy link
Collaborator Author

@hds @hawkw Do you have any thoughts about this problem? Maybe we should include the status in the state stream, or add a new API to allow clients to query the status?

@Rustin170506 Rustin170506 self-assigned this May 15, 2024
@hds
Copy link
Collaborator

hds commented May 15, 2024

I need to check the API, because I don't remember how it's set up currently. But ideally the status would be sent as part of the regular updates which are pushed from the console-subscriber server to tokio-console. This would serve as acknowledgement for the client that paused the session and would also inform other clients that the session has been paused and that they won't receive further updates until it is unpaused.

What do you think @hi-rustin?

@Rustin170506
Copy link
Collaborator Author

This would serve as acknowledgement for the client that paused the session and would also inform other clients that the session has been paused and that they won't receive further updates until it is unpaused.

Do you mean we always push the paused state on updates? If we only push it once, then the new client will not get it if it tries to connect to the server after the server has been paused.

@hds
Copy link
Collaborator

hds commented May 16, 2024

A new client should always receive at least one update, which would indicate that the state is "Paused" in this case.

@Rustin170506
Copy link
Collaborator Author

A new client should always receive at least one update, which would indicate that the state is "Paused" in this case.

Sounds good. I will try to add it.

@Rustin170506 Rustin170506 removed the C-api Crate: console-api label May 23, 2024
@Rustin170506 Rustin170506 mentioned this issue Jul 4, 2024
Closed
@Rustin170506 Rustin170506 mentioned this issue Sep 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Rustin170506 Rustin170506

Labels
C-console Crate: console. C-subscriber Crate: console-subscriber. S-bug Severity: bug
Projects
None yet
Milestone
No milestone
2 participants
@hds @Rustin170506