----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37381/#review95071 -----------------------------------------------------------
The code looks good. However, I don't think backward compatability is handled. What happens if I have been generating ledger ids based on ephemeral sequential and then move to the new method. I'll start from scratch again. It should fail on creating the ledger znode, but what if the ledger has just been deleted and all it's entries are still on bookies? Perhaps each ledgermanager type should provide it's a default ledger id generator, which the user can then override. - Ivan Kelly On Aug. 12, 2015, 2:21 a.m., Sijie Guo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37381/ > ----------------------------------------------------------- > > (Updated Aug. 12, 2015, 2:21 a.m.) > > > Review request for bookkeeper, fpj and Sijie Guo. > > > Bugs: BOOKKEEPER-438 > https://issues.apache.org/jira/browse/BOOKKEEPER-438 > > > Repository: bookkeeper-git > > > Description > ------- > > Move id generation out of LedgerManager to ensure different ledger manager > implementation shared same ledger id space in ZooKeeper, which is necessary > for migration between different ledger managers. > > > Diffs > ----- > > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BKException.java > c5be32d > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java > f74639b > > bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerCreateOp.java > 9cda8ca > > bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/AbstractConfiguration.java > 3ec2b5a > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java > f3f680d > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/CleanupLedgerManager.java > a7fbcf5 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/FlatLedgerManager.java > 2bc4258 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManager.java > 7f2df73 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/HierarchicalLedgerManagerFactory.java > b843e99 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerIdGenerator.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerManager.java > 7229028 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerManagerFactory.java > 7c3cf5c > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MSLedgerManagerFactory.java > 2510b89 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/ZkLedgerIdGenerator.java > PRE-CREATION > > bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java > 101fdac > > bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestWatchEnsembleChange.java > eb833a3 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/GcLedgersTest.java > 19aab44 > > bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/LedgerManagerTestCase.java > b95d2db > > bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/TestLedgerIdGenerator.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/37381/diff/ > > > Testing > ------- > > > Thanks, > > Sijie Guo > >
