> On 2011-01-24 09:45:33, Benjamin Reed wrote: > >
Thanks, Ben. Please have a look at my answers and let me know if they make sense to you. I'll then post an updated patch. > On 2011-01-24 09:45:33, Benjamin Reed wrote: > > trunk/src/contrib/bookkeeper/src/java/org/apache/bookkeeper/client/LedgerEntry.java, > > line 41 > > <https://reviews.apache.org/r/234/diff/1/?file=9382#file9382line41> > > > > does this variable get used? i can't find where. It is set in PendingReadOp.readEntryComplete, and we read it in LedgerRecoveryOp.readComplete. > On 2011-01-24 09:45:33, Benjamin Reed wrote: > > trunk/src/contrib/bookkeeper/src/java/org/apache/bookkeeper/client/LedgerMetadata.java, > > line 84 > > <https://reviews.apache.org/r/234/diff/1/?file=9384#file9384line84> > > > > isn't this supposed to be length += delta? (why didn't the test case > > catch it?) I forgot to remove that method, it is not exercised in this patch. > On 2011-01-24 09:45:33, Benjamin Reed wrote: > > trunk/src/contrib/bookkeeper/src/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java, > > line 133 > > <https://reviews.apache.org/r/234/diff/1/?file=9385#file9385line133> > > > > you need to comment why you are doing the subtraction here. Ok. > On 2011-01-24 09:45:33, Benjamin Reed wrote: > > trunk/src/contrib/bookkeeper/src/java/org/apache/bookkeeper/client/PendingReadOp.java, > > line 134 > > <https://reviews.apache.org/r/234/diff/1/?file=9386#file9386line134> > > > > we should be using the METADATA_LENGTH here right? METADATA_LENGTH is 32 bytes, while here we need 24. In reality, it is METADATA_LENGTH - 8. - fpj ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/234/#review148 ----------------------------------------------------------- On 2011-01-07 07:52:44, fpj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/234/ > ----------------------------------------------------------- > > (Updated 2011-01-07 07:52:44) > > > Review request for zookeeper, Dhruba Borthakur and Benjamin Reed. > > > Summary > ------- > > It is currently easy to know how many entries a ledger has, but there is no > easy way to know the total number of bytes in a ledger. The idea of this jira > is to add a method that gives the number of bytes in a closed ledger. My > current idea is to simply have the writer counting the number of bytes > written and store it to ZooKeeper. It is currently easy to know how many > entries a ledger has, but there is no easy way to know the total number of > bytes in a ledger. The idea of this jira is to add a method that gives the > number of bytes in a closed ledger. My current idea is to simply have the > writer counting the number of bytes written and store it to ZooKeeper. > > > Diffs > ----- > > > trunk/src/contrib/bookkeeper/src/java/org/apache/bookkeeper/client/DigestManager.java > 1055984 > > trunk/src/contrib/bookkeeper/src/java/org/apache/bookkeeper/client/LedgerEntry.java > 1055984 > > trunk/src/contrib/bookkeeper/src/java/org/apache/bookkeeper/client/LedgerHandle.java > 1055984 > > trunk/src/contrib/bookkeeper/src/java/org/apache/bookkeeper/client/LedgerMetadata.java > 1055984 > > trunk/src/contrib/bookkeeper/src/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java > 1055984 > > trunk/src/contrib/bookkeeper/src/java/org/apache/bookkeeper/client/PendingReadOp.java > 1055984 > > trunk/src/contrib/bookkeeper/src/java/org/apache/bookkeeper/tools/BookKeeperTools.java > 1055984 > > trunk/src/contrib/bookkeeper/test/org/apache/bookkeeper/test/AsyncLedgerOpsTest.java > 1055984 > > trunk/src/contrib/bookkeeper/test/org/apache/bookkeeper/test/BookieReadWriteTest.java > 1055984 > > trunk/src/contrib/bookkeeper/test/org/apache/bookkeeper/test/LedgerRecoveryTest.java > 1055984 > > Diff: https://reviews.apache.org/r/234/diff > > > Testing > ------- > > > Thanks, > > fpj > >
