Skip to content

Conversation

@saterus
Copy link
Contributor

@saterus saterus commented May 2, 2022

This fixes a bug which caused the shard supervisor to stall indefinitely. If it ran shards unassign and there was nothing to be unassigned, it would never increment the etcd revision with its transaction. Then when we waited to observe our changes as reflected by the etcd mod revision, we would end up waiting until the next arbitrary transaction to complete. Needless to say, this is not the desired behavior.

Also adds a timeout to the command more generally.


This change is Reviewable

@saterus saterus requested a review from jgraettinger May 2, 2022 17:40
@saterus saterus self-assigned this May 2, 2022
Alex Burkhart added 2 commits May 2, 2022 14:03
This fixes a bug which caused the shard supervisor to stall
indefinitely. If it ran `shards unassign` and there was nothing to be
unassigned, it would never increment the etcd revision with its
transaction. Then when we waited to observe our changes as reflected by
the etcd mod revision, we would end up waiting until the next arbitrary
transaction to complete. Needless to say, this is not the desired
behavior.
The underlying issue is now fixed, but this should prevent the `shards
unassign` command from hanging indefinitely in the future.
@saterus saterus force-pushed the alex/shard-unassign-hanging branch from d392871 to 833a615 Compare May 2, 2022 18:03
@saterus
Copy link
Contributor Author

saterus commented Jun 22, 2022

@jgraettinger Is this good to go out?

Copy link
Contributor

@jgraettinger jgraettinger left a comment

Choose a reason for hiding this comment

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

Yes, apologies. LGTM

@jgraettinger jgraettinger merged commit 836e403 into master Aug 25, 2022
@jgraettinger jgraettinger deleted the alex/shard-unassign-hanging branch August 25, 2022 13:28
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.

3 participants