> 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. > > Rakesh R wrote: > yes. I will do it. > > Rakesh R wrote: > I feel latch is simple to control the outerloop. What do you say? > > Ivan Kelly wrote: > but it's unnecessary. submit() returns a future. you just need to call > .get() on that to have the exact same effect.
oops, I missed it. Will do:) > On Dec. 9, 2014, 12:08 p.m., Ivan Kelly wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieShell.java, > > line 1199 > > <https://reviews.apache.org/r/28835/diff/1/?file=786288#file786288line1199> > > > > what does 0 mean with regards to printing? should be (default false). > > Rakesh R wrote: > Here I am taking option value as 'printMsgCount', using this I'm > calculate the printing cycle. For example the value 100 means. After every > 100, 200, 300... ledgers given for updation will print the message. > > Ivan Kelly wrote: > This isn't clear to the user. Perhaps it would also be better to print > every 5 seconds or so, rather than every X updates. Really what we want to > show to the user is that the process is still working. > > Rakesh R wrote: > If there are millions of ledgers, would be too noisy. Initially I had > done periodically,but modified later based on Sijie's concerns. > You want me to introduce one more parameter instead of reading the > optionvalue from verbose, like 'printmessageon' counter value and print based > on that? > > Ivan Kelly wrote: > I'd definitely make verbose and the period a separate parameter. -v, > --verbose are pretty standard parameters in command line tools. They normally > never take an argument. I'd either rename 'verbose' to 'printprogress' or > make 'verbose' turn output on/off and then use 'printprogress' to specify how > much. I prefer the first option, because you generally want some output at > least. > > Also, I don't think printing every 5 seconds is noisy. Noise is when > there's too much info to read any of it. One line every 5 seconds is easy to > handle. And it gives the user a hint that the process is progressing. I agree to provide verbose option [on/off] and introduce one more option 'printprogress'. This will have the oprion valie to print the frequency of the message. Since Sijie had given a comment earlier, it would be really great if Sijie can give a reply. Thanks! Sijie, do you agree to print messages on every configured seconds? - Rakesh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28835/#review64360 ----------------------------------------------------------- On Dec. 16, 2014, 4:07 a.m., Rakesh R wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28835/ > ----------------------------------------------------------- > > (Updated Dec. 16, 2014, 4:07 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/bookie/BookieShell.java > a1e4639 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/UpdateLedgerOp.java > PRE-CREATION > > 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 > >
