> On Dec. 9, 2014, 12:08 p.m., Ivan Kelly wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieShell.java, > > line 1198 > > <https://reviews.apache.org/r/28835/diff/1/?file=786288#file786288line1198> > > > > updation -> update > > > > In fact I would reword. > > > > Maximum number of ledgers to update (default: no limit).
k. I'll do it. > On Dec. 9, 2014, 12:08 p.m., Ivan Kelly wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieShell.java, > > line 1196 > > <https://reviews.apache.org/r/28835/diff/1/?file=786288#file786288line1196> > > > > 'no:' should be 'no.' ok. I'll do it > On Dec. 9, 2014, 12:08 p.m., Ivan Kelly wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieShell.java, > > line 1195 > > <https://reviews.apache.org/r/28835/diff/1/?file=786288#file786288line1195> > > > > long option should be updatesPerSec. Bandwidth is confusing in this > > context. ok, I will use as "updatespersec" > On Dec. 9, 2014, 12:08 p.m., Ivan Kelly wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/admin/UpdateLedgerOp.java, > > line 125 > > <https://reviews.apache.org/r/28835/diff/1/?file=786287#file786287line125> > > > > It's a little ugly to mix business logic and presentation. I think it > > would be better to pass in a notifier/callback type like. > > > > interface BlahNotifier { > > void progress(long updated, long issued); > > } > > > > And then have BookieShell pass in an implementaiton of this. ok. let me try. > On Dec. 9, 2014, 12:08 p.m., Ivan Kelly wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/admin/UpdateLedgerOp.java, > > line 83 > > <https://reviews.apache.org/r/28835/diff/1/?file=786287#file786287line83> > > > > updateCb would be better than metaCb. there's a lot of metadata ops > > happening. > > > > Also, create it inside #run(). It doesn't need to be this far up in the > > scope. ok, i will rename it. I think I need to keep it outside #run() as it will wait for the completion like: long ledgerUpdatedCount = updateCb.get(); isn't it? > On Dec. 9, 2014, 12:08 p.m., Ivan Kelly wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/admin/UpdateLedgerOp.java, > > line 82 > > <https://reviews.apache.org/r/28835/diff/1/?file=786287#file786287line82> > > > > Latch is not needed. The executor will return a future. just call > > .get() on it to get the same behaviour as the latch. yes. I will do it. - Rakesh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28835/#review64360 ----------------------------------------------------------- On Dec. 9, 2014, 4:05 a.m., Rakesh R wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28835/ > ----------------------------------------------------------- > > (Updated Dec. 9, 2014, 4:05 a.m.) > > > Review request for bookkeeper, fpj, Ivan Kelly, and Sijie Guo. > > > Bugs: BOKKEEPER-634 > https://issues.apache.org/jira/browse/BOKKEEPER-634 > > > Repository: bookkeeper-git > > > Description > ------- > > admin tool for changing the bookie's identifier to IP/hostname > > > Diffs > ----- > > > bookkeeper-server/src/main/java/org/apache/bookkeeper/admin/UpdateLedgerOp.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieShell.java > a1e4639 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java > 49d8e59 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/client/UpdateLedgerOpTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/28835/diff/ > > > Testing > ------- > > Added few tests > > > Thanks, > > Rakesh R > >
