Skip to content
New issue

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

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

Already on GitHub? # to your account

Stop publication cache task on disconnected channels #1676

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

Conversation

fuzzypixelz
Copy link
Member

While debugging ZettaScaleLabs/rmw_zenoh#44, GDB backtraces showed that the Net runtime was apparently blocked executing this publication cache task; applying this patch solved the issue.

Even though the blocking behavior I observed might've been a consequence of different underlying issue, I think this patch should be applied anyway. This is because:

  1. If I understand correctly, the tokio::select! macro doesn't execute poll each branch on every iteration, but rather uses a random number generator to choose the branch.
  2. Errors should be handled ;)

@fuzzypixelz fuzzypixelz added the enhancement Existing things could work better label Dec 17, 2024
@fuzzypixelz fuzzypixelz self-assigned this Dec 17, 2024
@fuzzypixelz fuzzypixelz force-pushed the publication-cache-early-return branch from 2696c9d to 7841ae3 Compare December 17, 2024 13:46
@fuzzypixelz fuzzypixelz force-pushed the publication-cache-early-return branch from 7841ae3 to 26481e7 Compare December 17, 2024 14:12
@Mallets
Copy link
Member

Mallets commented Feb 10, 2025

Advanced Pub/Sub is now available in main.
@fuzzypixelz is this PR still relevant?

@fuzzypixelz
Copy link
Member Author

Advanced Pub/Sub is now available in main. @fuzzypixelz is this PR still relevant?

I think so, I don't think that the publication cache code has changed.

@Mallets
Copy link
Member

Mallets commented Feb 10, 2025

Is the plan to merge it anyways? Knowing that querying subscriber has been marked as deprecated?

@fuzzypixelz fuzzypixelz marked this pull request as draft February 11, 2025 14:03
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement Existing things could work better
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants