You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bookkeeper.apache.org by Ivan Kelly <iv...@apache.org> on 2012/10/22 16:11:45 UTC
Re: Review Request: [BOOKKEEPER-204] Provide a MetaStore interface,
and a mock implementation.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7314/#review12653
-----------------------------------------------------------
bookkeeper-server/pom.xml
<https://reviews.apache.org/r/7314/#comment26988>
bk-server has added guava recently, so this is unnecessary.
bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/MetastoreCursor.java
<https://reviews.apache.org/r/7314/#comment26990>
this also needs a method to notify that there are no more entries.
bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/MetastoreScannableTable.java
<https://reviews.apache.org/r/7314/#comment26986>
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.
bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/MetastoreTable.java
<https://reviews.apache.org/r/7314/#comment26983>
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.
bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/MetastoreTable.java
<https://reviews.apache.org/r/7314/#comment26984>
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.
bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/MetastoreTableItem.java
<https://reviews.apache.org/r/7314/#comment26985>
you cant have copyright notices in ASF code
bookkeeper-server/src/main/java/org/apache/bookkeeper/metastore/Value.java
<https://reviews.apache.org/r/7314/#comment26987>
value should only be a byte[]. adding fields like this does, overexpands the scope of the change without a strong need.
- Ivan Kelly
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
>
>
Re: Review Request: [BOOKKEEPER-204] Provide a MetaStore interface,
and a mock implementation.
Posted by Ivan Kelly <iv...@apache.org>.
> 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.
>
> Jiannan Wang wrote:
> 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.
>
> Ivan Kelly wrote:
> This is a lot of complexity to add for a single corner case. A simpler solution would be to split SubscriptionData before writing to the metadata interface and to write to 2 separate keys, subid-prefs and subid-state for example.
>
>
> Sijie Guo wrote:
> Added my comments of fields:
>
> 1) As Jiannan described, separated infrequent and frequent data in different fields is one of the considerations.
> 2) In table system metadata storage, a field is required when adding record to a table.
>
> Jiannan Wang wrote:
> Yeah, split preference and state informations into two separate keys makes the API more simple. However, 1) it doubles the records. 2) separate into 2 keys means two different versions, and we may need more logic if we want to update both of them at the same time.
1) isnt really a problem
for 2), some stores will have multiple versions anyhow if you put them in separate columns (hbase for example).
Do subscription prefs ever change after the subscription has been created?
- Ivan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7314/#review12653
-----------------------------------------------------------
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
>
>
Re: Review Request: [BOOKKEEPER-204] Provide a MetaStore interface,
and a mock implementation.
Posted by Jiannan Wang <ji...@yahoo-inc.com>.
> 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
>
>
Re: Review Request: [BOOKKEEPER-204] Provide a MetaStore interface,
and a mock implementation.
Posted by Jiannan Wang <ji...@yahoo-inc.com>.
> 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.
>
> Jiannan Wang wrote:
> 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.
>
> Ivan Kelly wrote:
> This is a lot of complexity to add for a single corner case. A simpler solution would be to split SubscriptionData before writing to the metadata interface and to write to 2 separate keys, subid-prefs and subid-state for example.
>
>
> Sijie Guo wrote:
> Added my comments of fields:
>
> 1) As Jiannan described, separated infrequent and frequent data in different fields is one of the considerations.
> 2) In table system metadata storage, a field is required when adding record to a table.
Yeah, split preference and state informations into two separate keys makes the API more simple. However, 1) it doubles the records. 2) separate into 2 keys means two different versions, and we may need more logic if we want to update both of them at the same time.
- Jiannan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7314/#review12653
-----------------------------------------------------------
On Oct. 24, 2012, 12:07 p.m., Jiannan Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7314/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2012, 12:07 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
>
>
Re: Review Request: [BOOKKEEPER-204] Provide a MetaStore interface,
and a mock implementation.
Posted by Sijie Guo <gu...@gmail.com>.
> 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.
>
> Jiannan Wang wrote:
> 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.
>
> Ivan Kelly wrote:
> This is a lot of complexity to add for a single corner case. A simpler solution would be to split SubscriptionData before writing to the metadata interface and to write to 2 separate keys, subid-prefs and subid-state for example.
>
>
> Sijie Guo wrote:
> Added my comments of fields:
>
> 1) As Jiannan described, separated infrequent and frequent data in different fields is one of the considerations.
> 2) In table system metadata storage, a field is required when adding record to a table.
>
> Jiannan Wang wrote:
> Yeah, split preference and state informations into two separate keys makes the API more simple. However, 1) it doubles the records. 2) separate into 2 keys means two different versions, and we may need more logic if we want to update both of them at the same time.
>
> Ivan Kelly wrote:
> 1) isnt really a problem
> for 2), some stores will have multiple versions anyhow if you put them in separate columns (hbase for example).
> Do subscription prefs ever change after the subscription has been created?
> Do subscription prefs ever change after the subscription has been created?
In most cases, it doesn't change. But we allow the subscriber to change its preferences when it attached again to its subscription.
w/o fields in the value, you have to force all implementations to use the storage as pure key/value storage, which could not leverage the underlying storage's features.
w/ fields in the value, for a table system implementing such interface, it was easy to do the schema mapping. for a K/V system implementing such interface, it could generate its real key using the combination with given key and its field.
- Sijie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7314/#review12653
-----------------------------------------------------------
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
>
>
Re: Review Request: [BOOKKEEPER-204] Provide a MetaStore interface,
and a mock implementation.
Posted by Sijie Guo <gu...@gmail.com>.
> 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.
>
> Jiannan Wang wrote:
> 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.
>
> Ivan Kelly wrote:
> This is a lot of complexity to add for a single corner case. A simpler solution would be to split SubscriptionData before writing to the metadata interface and to write to 2 separate keys, subid-prefs and subid-state for example.
>
Added my comments of fields:
1) As Jiannan described, separated infrequent and frequent data in different fields is one of the considerations.
2) In table system metadata storage, a field is required when adding record to a table.
- Sijie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7314/#review12653
-----------------------------------------------------------
On Oct. 24, 2012, 12:07 p.m., Jiannan Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7314/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2012, 12:07 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
>
>
Re: Review Request: [BOOKKEEPER-204] Provide a MetaStore interface,
and a mock implementation.
Posted by Ivan Kelly <iv...@apache.org>.
> 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.
>
> Jiannan Wang wrote:
> 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.
This is a lot of complexity to add for a single corner case. A simpler solution would be to split SubscriptionData before writing to the metadata interface and to write to 2 separate keys, subid-prefs and subid-state for example.
- Ivan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7314/#review12653
-----------------------------------------------------------
On Oct. 24, 2012, 12:07 p.m., Jiannan Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7314/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2012, 12:07 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
>
>