----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33061/#review79997 -----------------------------------------------------------
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogMetadata.java <https://reviews.apache.org/r/33061/#comment129731> make it private bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogMetadata.java <https://reviews.apache.org/r/33061/#comment129732> remove trailing spaces? bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogMetadata.java <https://reviews.apache.org/r/33061/#comment129733> remove trailing spaces? bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogMetadata.java <https://reviews.apache.org/r/33061/#comment129734> remove trailing spaces? bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogMetadata.java <https://reviews.apache.org/r/33061/#comment129735> remove trailing spaces? bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java <https://reviews.apache.org/r/33061/#comment129736> I don't prefer using a int here. so for format upgrading, the old code could still read new version format by ignoring new metadata entry introduced in newer version. which it would be much rollback friendly. bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java <https://reviews.apache.org/r/33061/#comment129737> trailing space? bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java <https://reviews.apache.org/r/33061/#comment129738> I think you need to add a 4 bytes integer for total number of ledgers in the entry logger. so it would prevent any missing metadata entry causing missing ledgers. ledger map offset: 8 bytes total number of ledgers: 4 bytes bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java <https://reviews.apache.org/r/33061/#comment129739> trailing space? bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java <https://reviews.apache.org/r/33061/#comment129741> could you move #appendLedgerMap() to flushRotatedLogs()? so appendLedgerMap happens only on checkpointing, rather than when rolling log. because rolling log happens in critical path which will impact latency. bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java <https://reviews.apache.org/r/33061/#comment129743> append ledger map, should be appending multiple ledger maps metadata entries. rather than dumping into one big entry. --> (offset of ledgers map) - <ledgers_map_entry1> - <ledgers_map_entry2> - ... - <ledgers_map_entry3> bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java <https://reviews.apache.org/r/33061/#comment129744> remove this. to make it rollback friendly. as what the comment for HeaderVersion bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java <https://reviews.apache.org/r/33061/#comment129746> I don't think you need to limit maxPosition here, because the ledgers map entries will be skipped anyway. bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java <https://reviews.apache.org/r/33061/#comment129747> we shouldn't limit version. for future version upgrade. - Sijie Guo On April 10, 2015, 3:41 a.m., Matteo Merli wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33061/ > ----------------------------------------------------------- > > (Updated April 10, 2015, 3:41 a.m.) > > > Review request for bookkeeper. > > > Bugs: BOOKKEEPER-841 > https://issues.apache.org/jira/browse/BOOKKEEPER-841 > > > Repository: bookkeeper-git > > > Description > ------- > > Bookie should calculate ledgers map when writing a new entry log file. so the > bookie doesn't need to scan that entry log file again, which it would improve > garbage collection efficiency > > > Diffs > ----- > > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogMetadata.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java > 0e052b5 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java > 299fb3e > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java > 0a3884c > > bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/EntryLogTest.java > 488f4bf > > Diff: https://reviews.apache.org/r/33061/diff/ > > > Testing > ------- > > > Thanks, > > Matteo Merli > >
