[bookie-shell] fix readLedger with parameter -bookie and -ledgerid can not read any entries in ledger #3874
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
horizonzy
reviewed
Mar 19, 2023
|
|
||
| try { | ||
| future.get(); | ||
| } catch (Exception e) { | ||
| LOG.error("Error future.get while reading entries from ledger {}", flags.ledgerId, e); | ||
| if (e.getCause() instanceof BKException.BKNoSuchEntryException && lastEntry == -1) { |
There was a problem hiding this comment.
If the ledger only has 100 entries, but the input param lastEntry is 10000, it will still read 100-10000 entries.
There are two ways to solve it.
- remove the
lastEntry == -1judgment, once meet BKNoSuchEntryException, end the loop. - Add more limitations for the loop. (lastEntry == -1 || nextEntryId <= lastEntry && nextEntryId <= LAC), we can use bookieClient.readLac() to get the LAC.
|
@TakaHiR07 Are you still working on this? |
Sorry for forget this work. If needing, I can finish this in a few day. |
|
@TakaHiR07 Thanks for your contribution, I want to move this forward in the next release. |
TakaHiR07
force-pushed
the
improve_bookie_shell_readledger
branch
from
8baa5f7
to
99aaf0d
Compare
@shoothzj @horizonzy Have done. Can you help review again. Thx |
shoothzj
approved these changes
May 15, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
If we use bookieshell readLedger only with parameter -ledgerid, we can read all entries in this ledger, but if use with parameter -ledgerid and --bookie, we cant read any entries.
This is because it try to reverse entries from firstentry(default 0) to lastEntry(default -1). Therefore it would directly finish without read any entries, misleading us that ledgerId in our selected bookie has no entry.
Changes
when set -ledgerid and -bookie and not set -entry, continue to read next entry until get BKNoSuchEntryException