> On 2011-11-09 17:48:34, fpj wrote: > > Good job, Sijie. I only have a few minor comments. One important general > > comment I have is that we need documentation for this feature. Ideally we > > commit a patch for BOOKKEEPER-82 that includes doc changes. Otherwise, I'd > > like to have a blocker jira to make sure that we don't have a release > > without the doc changes.
I would add doc for this feature. > On 2011-11-09 17:48:34, fpj wrote: > > http://svn.apache.org/repos/asf/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java, > > line 592 > > <https://reviews.apache.org/r/2745/diff/1/?file=56625#file56625line592> > > > > I think we can remove this todo, right? Yes. > On 2011-11-09 17:48:34, fpj wrote: > > http://svn.apache.org/repos/asf/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java, > > line 598 > > <https://reviews.apache.org/r/2745/diff/1/?file=56625#file56625line598> > > > > Typo on journal. I will fix it. > On 2011-11-09 17:48:34, fpj wrote: > > http://svn.apache.org/repos/asf/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java, > > line 460 > > <https://reviews.apache.org/r/2745/diff/1/?file=56626#file56626line460> > > > > If there are corrupted entries, it is true that we can fetch copies > > from other bookies, but I don't think we have a mechanism for doing it yet, > > so it sounds safer to keep throwing an exception here for now, no? I changed throwing an exception to skipping it, because I found bookie servers can't be started due to entry logger corruption after restartBookieServers in test cases. This corruption is because that entry logger uses a BufferedChannel but it doesn't do flush when shutting down. So we read partial entry after those servers are restarted. I will add code in EntryLogger#shutdown to ensure buffered data persisted to disk, so it can pass unit tests. Since corruption is a complex issue, I agreed keeping throwing an exception here for now. I created a jira BOOKKEEPER-62 before. Maybe we can handle data corruption in that jira. - Sijie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2745/#review3130 ----------------------------------------------------------- On 2011-11-07 17:46:11, Sijie Guo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/2745/ > ----------------------------------------------------------- > > (Updated 2011-11-07 17:46:11) > > > Review request for bookkeeper. > > > Summary > ------- > > implement journal rolling > > 1) remove journals whose id are less than marked journal id. since they > journals' data and index has been persisted to disk. > 2) replay entires started from marked position in marked journal to speed up > bookie start up. > > > This addresses bug BOOKKEEPER-82. > http://issues.apache.org/jira/browse/BOOKKEEPER-82 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java > 1198776 > > http://svn.apache.org/repos/asf/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java > 1198776 > > http://svn.apache.org/repos/asf/zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieJournalRollingTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/2745/diff > > > Testing > ------- > > > Thanks, > > Sijie > >
