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

Fix race condition in LedgerHandle in drainPendingAddsAndAdjustLength & sendAddSuccessCallbacks #4175

Closed
wants to merge 3 commits into from

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented Jan 11, 2024

Motivation

There's a race condition in LedgerHandle between drainPendingAddsAndAdjustLength & sendAddSuccessCallbacks method calls. The methods could get called simultaneously which would cause inconsistency.
The changingEnsemble field has a thread safety issue and the changes might not be visible to the reading thread.

Changes

  • use synchronized (pendingAddOps) as the object monitor for serializing drainPendingAddsAndAdjustLength & sendAddSuccessCallbacks method calls.
  • make changingEnsemble field volatile
  • remove the too strict rule for pendingAddsSequenceHead which breaks things after failures

This rule is removed:

// Check if it is the next entry in the sequence.
if (pendingAddOp.entryId != 0 && pendingAddOp.entryId != pendingAddsSequenceHead + 1) {
if (LOG.isDebugEnabled()) {
LOG.debug("Head of the queue entryId: {} is not the expected value: {}", pendingAddOp.entryId,
pendingAddsSequenceHead + 1);
}
return;
}

The current assumption is that this doesn't hold under failure conditions.

Additional Context

@lhotari
Copy link
Member Author

lhotari commented Jan 25, 2024

Closing in favor of the original PR #4171.

@lhotari lhotari closed this Jan 25, 2024
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.

1 participant