> On Oct. 22, 2012, 2:11 p.m., Ivan Kelly wrote: > > bookkeeper-server/pom.xml, line 95 > > <https://reviews.apache.org/r/7314/diff/1/?file=160299#file160299line95> > > > > bk-server has added guava recently, so this is unnecessary.
will rebase patch to latest trunk > On Oct. 22, 2012, 2:11 p.m., Ivan Kelly wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/MetastoreCursor.java, > > line 53 > > <https://reviews.apache.org/r/7314/diff/1/?file=160303#file160303line53> > > > > this also needs a method to notify that there are no more entries. MetastoreCursor#hasMoreEntries() has already indicate there is no more entry. > On Oct. 22, 2012, 2:11 p.m., Ivan Kelly wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/MetastoreTable.java, > > line 100 > > <https://reviews.apache.org/r/7314/diff/1/?file=160307#file160307line100> > > > > i dont think we need a put and a versionedPut. I think every write > > should be versionde. in the past, when they have not been, we have run into > > cases where we found that they really should be versioned. Yeah, will remove put interface. > On Oct. 22, 2012, 2:11 p.m., Ivan Kelly wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/MetastoreTable.java, > > line 129 > > <https://reviews.apache.org/r/7314/diff/1/?file=160307#file160307line129> > > > > The use of VersionedValue here is a little strange. The Version is the > > version of the data that is in the store, the value is new value you want > > to write. as such, they should be two separate parameters. will fix > On Oct. 22, 2012, 2:11 p.m., Ivan Kelly wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/MetastoreTableItem.java, > > line 2 > > <https://reviews.apache.org/r/7314/diff/1/?file=160308#file160308line2> > > > > you cant have copyright notices in ASF code I'll change the header > On Oct. 22, 2012, 2:11 p.m., Ivan Kelly wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/MetastoreScannableTable.java, > > line 25 > > <https://reviews.apache.org/r/7314/diff/1/?file=160306#file160306line25> > > > > why do we have scannable table and normal table? what happens if a > > client expects a scannable table and the implementation doesnt support it? > > I suggest there should be only one type of table, otherwise the contract we > > provide to the client is ambiguous. The scannable table is required to support scan operation, which takes place in: 1) Get all subscriptions of topic 2) Garbage collection on deleted ledgers We can consider to remove Scannable table after these two scan usage are removed (the former is BOOKKEEPER-412 and the latte will be addressed by a new GC algorithm in BOOKKEEPER-249). > On Oct. 22, 2012, 2:11 p.m., Ivan Kelly wrote: > > bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/Value.java, > > line 37 > > <https://reviews.apache.org/r/7314/diff/1/?file=160309#file160309line37> > > > > value should only be a byte[]. adding fields like this does, > > overexpands the scope of the change without a strong need. Currently, SubscriptionData contains preference and state information, where state is updated frequently while preference changes only when sub. To make better performance, SubscriptionDataManager supports partial update operation. This is the reason why we introduce fields in Value to support updating a specific field. - Jiannan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7314/#review12653 ----------------------------------------------------------- On Sept. 27, 2012, 9:21 a.m., Jiannan Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7314/ > ----------------------------------------------------------- > > (Updated Sept. 27, 2012, 9:21 a.m.) > > > Review request for bookkeeper. > > > Description > ------- > > We need a MetaStore interface which easy for us to plugin different scalable > k/v storage, such as HBase. > > > This addresses bug BOOKKEEPER-204. > https://issues.apache.org/jira/browse/BOOKKEEPER-204 > > > Diffs > ----- > > bookkeeper-server/pom.xml c608361 > > bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/MSException.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/MetaStore.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/MetastoreCallback.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/MetastoreCursor.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/MetastoreException.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/MetastoreFactory.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/MetastoreScannableTable.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/MetastoreTable.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/MetastoreTableItem.java > PRE-CREATION > bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/Value.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/VersionedValue.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/mock/MockMetaStore.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/mock/MockMetastoreCursor.java > PRE-CREATION > > bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/mock/MockMetastoreTable.java > PRE-CREATION > > bookkeeper-server/src/test/java/org/apache/bookkeeper/metastore/MetastoreScannableTableAsyncToSyncConverter.java > PRE-CREATION > > bookkeeper-server/src/test/java/org/apache/bookkeeper/metastore/MetastoreTableAsyncToSyncConverter.java > PRE-CREATION > > bookkeeper-server/src/test/java/org/apache/bookkeeper/metastore/TestMetaStore.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/7314/diff/ > > > Testing > ------- > > > Thanks, > > Jiannan Wang > >
