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.

Sign up for GitHub

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

Jump to bottom

[fix][broker]Ensure that the ledgerId updated by currentLedger is monotonically increasing #16647

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[fix][broker]Ensure that the ledgerId updated by currentLedger is monotonically increasing #16647

wants to merge 1 commit into from

Conversation

lordcheng10
Copy link
Contributor

Motivation

When updating the currentLedger, it is necessary to ensure that the updated ledgerId is greater than the ledgerId of the current currentLedger.

Documentation

Check the box below or label this PR directly.

Need to update docs?

  • doc-required
    (Your PR needs to update docs and you will update later)

  • doc-not-needed
    (Please explain why)

  • doc
    (Your PR contains doc changes)

  • doc-complete
    (Docs have been already added)

@lordcheng10
Copy link
Contributor Author

/pulsarbot run-failure-checks

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jul 18, 2022
@lordcheng10
Copy link
Contributor Author

/pulsarbot run-failure-checks

@mattisonchao
Copy link
Member

@lordcheng10
Looks like we have the lock to ensure concurrency problem, could you describe the detail?

@lordcheng10
Copy link
Contributor Author

lordcheng10 commented Jul 29, 2022 •
edited
Loading

@lordcheng10 Looks like we have the lock to ensure concurrency problem, could you describe the detail?

@mattisonchao When there is a serious Full GC, there may be multiple ledgers to update the metadata information on zookeeper at the same time. Suppose we create ledger in the order of ledger1, ledger2, ledger3, ledger4, ledger5, and assume that a comparison occurs. In severe Full GC, ledger5 may grab the metadataMutex lock before ledger4, so ledger5 may trigger the zookeeper callback method operationComplete before ledger4, so that ledger4 may cover ledger5:

pulsar/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java

Lines 1559 to 1566 in 4d64e2e

private void updateLedgersListAfterRollover(MetaStoreCallback<Void> callback, LedgerInfo newLedger) {
if (!metadataMutex.tryLock()) {
// Defer update for later
scheduledExecutor.schedule(() -> updateLedgersListAfterRollover(callback, newLedger),
100, TimeUnit.MILLISECONDS);
return;
}

@lordcheng10
Copy link
Contributor Author

@codelipenghui @hangc0276 PTAL,thanks!

@github-actions
Copy link

github-actions bot commented Sep 3, 2022

The pr had no activity for 30 days, mark with Stale label.

@github-actions github-actions bot added the Stale label Sep 3, 2022
@Technoboy- Technoboy- added this to the 3.2.0 milestone Jul 31, 2023
@Technoboy- Technoboy- modified the milestones: 3.2.0, 3.3.0 Dec 22, 2023
@coderzc coderzc modified the milestones: 3.3.0, 3.4.0 May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@merlimat merlimat Awaiting requested review from merlimat

@BewareMyPower BewareMyPower Awaiting requested review from BewareMyPower

@codelipenghui codelipenghui Awaiting requested review from codelipenghui

@gaoran10 gaoran10 Awaiting requested review from gaoran10

@hangc0276 hangc0276 Awaiting requested review from hangc0276

@zymap zymap Awaiting requested review from zymap

@nodece nodece Awaiting requested review from nodece

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
doc-not-needed Your PR changes do not impact docs Stale
Projects
None yet
Milestone
4.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@lordcheng10 @mattisonchao @Technoboy- @coderzc