Support DbLedgerStorage in LedgerCmd to get list of logger files for a given ledgerId #6
Conversation
| } | ||
| } finally { | ||
| ledgersDb.close(); | ||
| } |
There was a problem hiding this comment.
This feels way too much like you are reaching into the internals of EntryLocationIndex. Even though we do very similar things in other parts of this same file, I really would prefer to at least see the internals of most of this moving to EntryLocationIndex.java, so the code that messes with this DB is all in the same file.
In fact it looks like you could use
for almost all of this, and no need for accessing ArrayUtil
There was a problem hiding this comment.
that's correct, I have implemented it earlier because we have to initialize rocksDB in read-only mode and we can do it by passing custom KeyValueStorageFactory so, we can reuse EntryLocationIndex. I have changed it to use EntryLocationIndex for getting ledger location.
| LOG.info("Create ledger and add entries to it"); | ||
| LedgerHandle lh1 = createLedgerWithEntries(bk, 10); | ||
|
|
||
| Thread.sleep(1000); // sleep to flush entries to logger file |
There was a problem hiding this comment.
Is there no other way to force it to sync?
There was a problem hiding this comment.
yes, removed it by decreasing flush time.
rdhabalia
force-pushed
the
tool_ledger
branch
3 times, most recently
from
2540c2d to
fd0367c
Compare
| long pos = offset & 0xffffffffL; | ||
| System.out.println("entry " + curEntry + "\t:\t(log: " + entryLogId + ", pos: " + pos + ")"); | ||
| } catch (Exception e) { | ||
| System.err.println("ERROR: while getting location for entry " + ledgerId + ", " + curEntry + ", " |
There was a problem hiding this comment.
A single bookie is not required to have all the entries for a particular ledger.
If you have e=3 w=2 a=2 for example, each bookie will be missing 1 entry out of 3, we shouldn't break the loop at that point.
Also, the first entry might be very after 0. Eg: when a bookie crashes (or times out), the client will keep using the same ledger and form a new ensemble by removing the old bookie and picking a new one.
The new bookie will only have entries from that point on.
There was a problem hiding this comment.
A single bookie is not required to have all the entries for a particular ledger.
Yes, in that case we should not break and continue. Fixed it,
| * @return | ||
| * @throws IOException if failed to init KeyValueStorageRocksDB | ||
| */ | ||
| protected int readLedgerIndexEntriesFromDbLedgerStorage(long ledgerId) throws IOException { |
There was a problem hiding this comment.
Minor thing: Since this is specific to DbLedgerStorage, should we have the code in that class too?
There was a problem hiding this comment.
actually DbLedgerStorage initializes multiple things other than EntryLocationIndex such as
LedgerMetadataIndex, GC, EntryLogger. And also, this cmd requires EntryLocationIndex instance which starts rocksDB in readOnly mode by passing custom KeyValueStorageFactory factory. So, I think it's bettter to keep this code in Cmd rather adding different init method in DbStorage just for EntryLocationIndex . Any thoughts?
There was a problem hiding this comment.
Sure. I was thinking more like a static method in there or to add a class in the ..storage.ldb. package, just to keep all the DbLederStorage specific code contained in one place as much as possible.
rdhabalia
force-pushed
the
tool_ledger
branch
3 times, most recently
from
e1534a2 to
30da591
Compare
* Use statelib to store metadata * Remove namespace * Use local state store to store root range
Motivation
In certain conditions client may require to get list of entryLog files that contain given ledgerId for specific auditing or cleanup. Right now,
LedgerCmdunderBookieShellgives similar information but it doesn't support DbLedgerStorage so, added support for DbLedgerStorage to LedgerCmd.Modifications
Add DbLedgerStorage support under
BookieShell.LedgerCmd.