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

Create and use a separate iodispatcher for scavenge log #3741

Merged
merged 1 commit into from
Feb 27, 2023

Conversation

hayley-jean
Copy link
Member

Fixed: Use a separate IODispatcher for scavenge log

This fixes a bug introduced by #3702, which changed the IODispatcher used by persistent subscriptions to subscribe and publish to the psub-specific queues and buses.

However, the scavenge log was using the same IODispatcher, so the messages for scavenging were being sent to the wrong queue.

Copy link
Contributor

@timothycoleman timothycoleman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

For posterity, the risk of the IO Dispatcher publishing on to the mainqueue is that the registered callbacks will be called on the main bus which is dangerous if they aren't guaranteed to be quick. I've checked that the callbacks on this iodispatcher are quick and they all look fine except for TFChunkScavengerLogManager::GatherIncompleteScavenges which is potentially longer but only happens once on startup so I think this is fine for now

@hayley-jean hayley-jean merged commit 1077ad1 into master Feb 27, 2023
@hayley-jean hayley-jean deleted the hayley-jean/fix-scavenging-dispatcher branch February 27, 2023 09:50
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.

None yet

3 participants