Skip to content

Conversation

@lhotari
Copy link
Member

@lhotari lhotari commented Feb 17, 2025

Motivation

In some tests, I noticed that LeaderElectionImpl could continue to make operations and override the closed state.

Modifications

Add checks if the internalState is Closed

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@lhotari lhotari added this to the 4.1.0 milestone Feb 17, 2025
@lhotari lhotari self-assigned this Feb 17, 2025
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Feb 17, 2025
@lhotari lhotari force-pushed the lh-fix-LeaderElectionImpl-close branch from 2d4c72e to a62e10a Compare February 18, 2025 09:24
}
}).thenCompose(leaderElectionState -> {
synchronized (this) {
if (internalState == InternalState.Closed) {
Copy link
Member

Choose a reason for hiding this comment

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

Need to add volatile on internalState?

Copy link
Member Author

Choose a reason for hiding this comment

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

Already covered with synchronized (this). this points to LeaderElectionImpl instance here.

@lhotari lhotari requested a review from dao-jun February 18, 2025 13:57
@lhotari lhotari marked this pull request as draft February 18, 2025 13:58
@lhotari
Copy link
Member Author

lhotari commented Feb 18, 2025

Fixing a potentially flaky test that uses a bad way to trigger leader election.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants