> On Oct. 24, 2012, 1:23 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> > > > > My question is why is it a separate interface though? Do you ever see a > > case where MetastoreTable will be implemented and not > > MetastoreScannableTable? I think the two should be merged now, and if the > > need for scan is removed in the future we can remove it from all the > > interface. This isn't going to be a public interface. > > Sijie Guo wrote: > Added my comment: In some table system, there are two kinds for table, > HASH and ORDER. For topic ownership and topic persistence info, a HASH table > is enough. While GC and ReadSubscriptions, an ORDER table is required now. > The different between MetastoreTable and MetastoreScannableTable is > MetastoreScannableTable provides order scan. > > Jiannan Wang wrote: > As sijie's comment, some key-value storages make better optimization if > online scan operation is not required. And scan is only used by GC and > ReadSubscriptions, so we separate it. > > Ivan Kelly wrote: > If a metastore doesnt provide scan then it can't do GC & read > subscriptions. This makes that metastore unusable. This is my point. > Metastore is being done to provide a single metadata interface for hw + bk. > Scan is a necessary requirement for bk + hw, therefore a table interface that > doesn't use scan is of little use. > > Sijie Guo wrote: > in general, we have only two tables requires order table scan, one is > ledger metadata table, the other one is subscription metadata table. but the > table of persistence info and topic owner, which doesn't need a scan table. > And a HASH table is more suitable for the accesses to persistence and topic > owner tables. The interface of MetastoreTable also have scan interface, but > it doesn't require the scan to be in order, while the interface of > MetastoreScannableTable is based on MetastoreTable just adding order scan > requirements to indicating it was an order table. > > We don't want to force all tables to be order table, since order table > has its pros and cons.
Ok. What I don't like here, is that one specific database implementation is driving the design of the overall interface. But as you guys are the main driver for this change, it's not a big issue for me. I'll take another look over this change tomorrow and if there's nothing more, I'll push it in. - Ivan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7314/#review12712 ----------------------------------------------------------- On Nov. 1, 2012, 4:27 p.m., Jiannan Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7314/ > ----------------------------------------------------------- > > (Updated Nov. 1, 2012, 4:27 p.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/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/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 > >
