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 NoSuchElementException when rereplicate empty ledgers #4039

Merged
merged 4 commits into from
Sep 20, 2023

Conversation

chenhongSZ
Copy link
Contributor

@chenhongSZ chenhongSZ commented Jul 27, 2023

Motivation

Master issue: #4036

Changes

Set the numberOfEntriesToReplicate to 0 when ledger is empty

@aloyszhang
Copy link
Contributor

@hangc0276 @zymap PTAL

Copy link
Contributor

@hangc0276 hangc0276 left a comment

Choose a reason for hiding this comment

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

Nice catch. @chenhongSZ Would you please add a test to cover this change? thanks.

@chenhongSZ
Copy link
Contributor Author

chenhongSZ commented Aug 13, 2023

Nice catch. @chenhongSZ Would you please add a test to cover this change? thanks.

done, PTAL

Comment on lines 296 to 298
long numberOfEntriesToReplicate = firstEntryId == INVALID_ENTRY_ID && lastEntryId == INVALID_ENTRY_ID ? 0 :
(lastEntryId - firstEntryId) + 1;
long splitsWithFullEntries = numberOfEntriesToReplicate
Copy link
Member

Choose a reason for hiding this comment

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

Can you share the configuration of replicationEntryBatchSize in the bookie server?
If I understand correctly, this problem will only occur when replicationEntryBatchSize=1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Your understanding is correct. Our configuration is rereplicationEntryBatchSize=1

@@ -293,7 +293,8 @@ static Set<LedgerFragment> splitIntoSubFragments(LedgerHandle lh,
assert false;
}

long numberOfEntriesToReplicate = (lastEntryId - firstEntryId) + 1;
long numberOfEntriesToReplicate = firstEntryId == INVALID_ENTRY_ID && lastEntryId == INVALID_ENTRY_ID ? 0 :
Copy link
Contributor

Choose a reason for hiding this comment

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

We can simplify the condition to long numberOfEntriesToReplicate = firstEntryId == INVALID_ENTRY_ID ? 0 : (lastEntryId - firstEntryId) + 1;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, PTAL

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

@hangc0276 hangc0276 merged commit 95320f4 into apache:master Sep 20, 2023
25 of 26 checks passed
zymap pushed a commit that referenced this pull request Dec 7, 2023
### Motivation
Master issue: #4036

### Changes
Set the `numberOfEntriesToReplicate` to 0 when ledger is empty

(cherry picked from commit 95320f4)
hangc0276 pushed a commit to hangc0276/bookkeeper that referenced this pull request Jan 18, 2024
### Motivation
Master issue: apache#4036

### Changes
Set the `numberOfEntriesToReplicate` to 0 when ledger is empty

(cherry picked from commit 95320f4)
Ghatage pushed a commit to sijie/bookkeeper that referenced this pull request Jul 12, 2024
### Motivation
Master issue: apache#4036 

### Changes
Set the `numberOfEntriesToReplicate` to 0 when ledger is empty
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.

5 participants