[jira] [Issue Comment Edited] (HBASE-5229) Provide basic building blocks for multi-row local transactions.
[ https://issues.apache.org/jira/browse/HBASE-5229?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13199514#comment-13199514 ] Lars Hofhansl edited comment on HBASE-5229 at 2/9/12 9:15 PM: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3748/#review4788 --- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java https://reviews.apache.org/r/3748/#comment10525 Should be ' to mutateRows' http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java https://reviews.apache.org/r/3748/#comment10526 Should read ' the mutations' http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiRowMutation.java https://reviews.apache.org/r/3748/#comment10529 Can the two add() methods be combined into one which accepts Mutation ? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiRowMutation.java https://reviews.apache.org/r/3748/#comment10527 Is this method thread-safe ? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiRowMutation.java https://reviews.apache.org/r/3748/#comment10528 This comment can be removed, right ? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/RowMutation.java https://reviews.apache.org/r/3748/#comment10531 From its name, RowMutation seems to refer to single row. It is a little confusing RowMutation extends MultiRowMutation. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/RowMutation.java https://reviews.apache.org/r/3748/#comment10530 version would be read / written twice, right ? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java https://reviews.apache.org/r/3748/#comment10532 Should be 'within the region', right ? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java https://reviews.apache.org/r/3748/#comment10533 rowsToLock.size() could be smaller than mutations.size(), right ? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java https://reviews.apache.org/r/3748/#comment10535 Can we refer regionName from rm (the MultiRowMutation) ? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java https://reviews.apache.org/r/3748/#comment10534 Should be mutateRows http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/rest/client/RemoteHTable.java https://reviews.apache.org/r/3748/#comment10536 Should read atomic mutation - Ted was (Author: jirapos...@reviews.apache.org): --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3748/#review4788 --- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java https://reviews.apache.org/r/3748/#comment10525 Should be ' to mutateRows' http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java https://reviews.apache.org/r/3748/#comment10526 Should read ' the mutations' http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiRowMutation.java https://reviews.apache.org/r/3748/#comment10529 Can the two add() methods be combined into one which accepts Mutation ? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiRowMutation.java https://reviews.apache.org/r/3748/#comment10527 Is this method thread-safe ? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiRowMutation.java https://reviews.apache.org/r/3748/#comment10528 This comment can be removed, right ? http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/RowMutation.java https://reviews.apache.org/r/3748/#comment10531 From its name, RowMutation seems to refer to single row. It is a little confusing RowMutation extends MultiRowMutation. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/RowMutation.java https://reviews.apache.org/r/3748/#comment10530 version would be read / written twice, right ?
[jira] [Issue Comment Edited] (HBASE-5229) Provide basic building blocks for multi-row local transactions.
[ https://issues.apache.org/jira/browse/HBASE-5229?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13201354#comment-13201354 ] Lars Hofhansl edited comment on HBASE-5229 at 2/9/12 9:18 PM: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3748/#review4833 --- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java https://reviews.apache.org/r/3748/#comment10616 Do we need to be sorting rowsToLock? I'm thinking of multiple concurrent mutateRows operation, trying to lock the same set of rows. Perhaps, throwing IOException is going to prevent us from a situation where we end up with a deadlock. But, we still might want to sort it to ensure (better) progress (no livelock). - Amitanand was (Author: jirapos...@reviews.apache.org): --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3748/#review4833 --- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java https://reviews.apache.org/r/3748/#comment10616 Do we need to be sorting rowsToLock? I'm thinking of multiple concurrent mutateRows operation, trying to lock the same set of rows. Perhaps, throwing IOException is going to prevent us from a situation where we end up with a deadlock. But, we still might want to sort it to ensure (better) progress (no livelock). - Amitanand On 2012-02-03 19:59:55, Lars Hofhansl wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/3748/ bq. --- bq. bq. (Updated 2012-02-03 19:59:55) bq. bq. bq. Review request for hbase. bq. bq. bq. Summary bq. --- bq. bq. This builds on HBASE-3584, HBASE-5203, and HBASE-5304. bq. bq. Multiple Rows can be locked and applied atomically as long as the application ensures that all rows reside in the same Region (by presplitting or a custom RegionSplitPolicy). bq. At SFDC we can use this to colocate subsets of a tenant's data and allow atomic operations over these subsets. bq. bq. Obviously this is an advanced features and this prominently called out in the Javadoc. bq. bq. bq. This addresses bug HBASE-5229. bq. https://issues.apache.org/jira/browse/HBASE-5229 bq. bq. bq. Diffs bq. - bq. bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java 1239953 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java 1239953 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiRowMutation.java PRE-CREATION bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java 1239953 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java 1239953 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 1239953 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1239953 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1239953 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/rest/client/RemoteHTable.java 1239953 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java 1239953 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestAtomicOperation.java 1239953 bq. bq. Diff: https://reviews.apache.org/r/3748/diff bq. bq. bq. Testing bq. --- bq. bq. Tests added to TestFromClientSide and TestAtomicOperation bq. bq. bq. Thanks, bq. bq. Lars bq. bq. Provide basic building blocks for multi-row local transactions. - Key: HBASE-5229 URL: https://issues.apache.org/jira/browse/HBASE-5229 Project: HBase Issue Type: New Feature Components: client, regionserver Reporter: Lars Hofhansl Assignee: Lars Hofhansl Fix For: 0.94.0
[jira] [Issue Comment Edited] (HBASE-5229) Provide basic building blocks for multi-row local transactions.
[ https://issues.apache.org/jira/browse/HBASE-5229?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13200911#comment-13200911 ] Lars Hofhansl edited comment on HBASE-5229 at 2/9/12 9:17 PM: -- bq. On 2012-02-05 07:26:08, Jesse Yates wrote: bq. A couple of nits and small implementation details, but overall looks pretty good. You're looking at an old version of the patch. :) bq. On 2012-02-05 07:26:08, Jesse Yates wrote: bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java, line 3160 bq. https://reviews.apache.org/r/3748/diff/1/?file=72045#file72045line3160 bq. bq. But in the comments on the MultiRowMutation you push that checking off onto the RS, so no checking really happens then (except, I guess when you try to mutate rows on the region and it fails b/c those rows aren't there, but that seems kinda late for the check). Checking is happening the region.internalMutate. bq. On 2012-02-05 07:26:08, Jesse Yates wrote: bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java, line 786 bq. https://reviews.apache.org/r/3748/diff/1/?file=72037#file72037line786 bq. bq. I think is this unnecessary, javadoc should handle inheriting the docs. It's done elsewhere, it is good to call out that no doc was added here, because the interface has the doc. bq. On 2012-02-05 07:26:08, Jesse Yates wrote: bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java, line 284 bq. https://reviews.apache.org/r/3748/diff/1/?file=72038#file72038line284 bq. bq. or presplitting as is described in other documenttation. Yes, should add this. bq. On 2012-02-05 07:26:08, Jesse Yates wrote: bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiRowMutation.java, line 35 bq. https://reviews.apache.org/r/3748/diff/1/?file=72039#file72039line35 bq. bq. Probably want to wrap NOTE in b tags to call it out. Sure. bq. On 2012-02-05 07:26:08, Jesse Yates wrote: bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiRowMutation.java, line 45 bq. https://reviews.apache.org/r/3748/diff/1/?file=72039#file72039line45 bq. bq. A javadoc here might be nice to indicate that the nullary constructor is actually completely ok to use (as opposed to the more common state of being reserved for readFields). Good point. Although unless it is called out that you cannot use a constructor there should be no reason whyt you couldn't. bq. On 2012-02-05 07:26:08, Jesse Yates wrote: bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiRowMutation.java, line 64 bq. https://reviews.apache.org/r/3748/diff/1/?file=72039#file72039line64 bq. bq. Even though it uses protected structures doesn't mean that its necessarily thread safe. In fact, because it is using the standard ArrayList, there is no guarantee of safety. Either the class should be marked as not thread safe OR the mutations should be wrapped as a concurrent list. I disagree. This is a client side object and none of the client side objects are threadsafe nor should they be (see Put.java/Delete.java/Increment.java/Append.java/etc), that's the task of client application. I misread Ted's comment before, of course this method is not threadsafe. bq. On 2012-02-05 07:26:08, Jesse Yates wrote: bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/RowMutation.java, line 95 bq. https://reviews.apache.org/r/3748/diff/1/?file=72040#file72040line95 bq. bq. You really don't need to keep around the row anymore either because you can get that from the mutations as you already do mutateRows with MultiRowMutation. Its nice to store it, but is only going to be checked infrequently and saves you a little bit over the wire (which could add up, depending on row size). Same as Put and Delete (where every KV already has the row). There is room optimization, but this is not the jira to do that. bq. On 2012-02-05 07:26:08, Jesse Yates wrote: bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 4161 bq. https://reviews.apache.org/r/3748/diff/1/?file=72044#file72044line4161 bq. bq. Suprised this isn't a utility method in HRegion - it seems really useful. Maybe worth pulling out for general use. internalMutate? bq. On 2012-02-05 07:26:08, Jesse Yates wrote: bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 4181 bq. https://reviews.apache.org/r/3748/diff/1/?file=72044#file72044line4181 bq. bq. This
[jira] [Issue Comment Edited] (HBASE-5229) Provide basic building blocks for multi-row local transactions.
[ https://issues.apache.org/jira/browse/HBASE-5229?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13200681#comment-13200681 ] Lars Hofhansl edited comment on HBASE-5229 at 2/9/12 9:17 PM: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3748/#review4822 --- A couple of nits and small implementation details, but overall looks pretty good. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java https://reviews.apache.org/r/3748/#comment10588 I think is this unnecessary, javadoc should handle inheriting the docs. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java https://reviews.apache.org/r/3748/#comment10587 or presplitting as is described in other documenttation. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiRowMutation.java https://reviews.apache.org/r/3748/#comment10586 Probably want to wrap NOTE in b tags to call it out. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiRowMutation.java https://reviews.apache.org/r/3748/#comment10590 A javadoc here might be nice to indicate that the nullary constructor is actually completely ok to use (as opposed to the more common state of being reserved for readFields). http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiRowMutation.java https://reviews.apache.org/r/3748/#comment10585 Even though it uses protected structures doesn't mean that its necessarily thread safe. In fact, because it is using the standard ArrayList, there is no guarantee of safety. Either the class should be marked as not thread safe OR the mutations should be wrapped as a concurrent list. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/RowMutation.java https://reviews.apache.org/r/3748/#comment10591 You really don't need to keep around the row anymore either because you can get that from the mutations as you already do mutateRows with MultiRowMutation. Its nice to store it, but is only going to be checked infrequently and saves you a little bit over the wire (which could add up, depending on row size). http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java https://reviews.apache.org/r/3748/#comment10592 Suprised this isn't a utility method in HRegion - it seems really useful. Maybe worth pulling out for general use. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java https://reviews.apache.org/r/3748/#comment10593 This isn't actually true, right? With multirow, you are actually going to lock more than one row (and the lockId null seems kind of a hack around that as it is always null, so far). http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java https://reviews.apache.org/r/3748/#comment10594 nit: lockID rather than just lid would be slightly descriptive. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java https://reviews.apache.org/r/3748/#comment10595 But in the comments on the MultiRowMutation you push that checking off onto the RS, so no checking really happens then (except, I guess when you try to mutate rows on the region and it fails b/c those rows aren't there, but that seems kinda late for the check). http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java https://reviews.apache.org/r/3748/#comment10596 Wow, this is ugly. Maybe we should consider some refactoring of this later? http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestAtomicOperation.java https://reviews.apache.org/r/3748/#comment10597 This class can get easily bloated as we add more types. Might be worth considering refactoring this out into its own test. - Jesse was (Author: jirapos...@reviews.apache.org): --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3748/#review4822 --- A couple of nits and small implementation details, but overall looks pretty good. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java https://reviews.apache.org/r/3748/#comment10588 I think is this unnecessary, javadoc should handle
[jira] [Issue Comment Edited] (HBASE-5229) Provide basic building blocks for multi-row local transactions.
[ https://issues.apache.org/jira/browse/HBASE-5229?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13201365#comment-13201365 ] Lars Hofhansl edited comment on HBASE-5229 at 2/9/12 9:18 PM: -- bq. On 2012-02-06 15:52:43, Amitanand Aiyer wrote: bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 4212 bq. https://reviews.apache.org/r/3748/diff/2/?file=72266#file72266line4212 bq. bq. Do we need to be sorting rowsToLock? bq. bq. I'm thinking of multiple concurrent mutateRows operation, trying to lock the same set of rows. bq. bq. Perhaps, throwing IOException is going to prevent us from a situation where we end up with a deadlock. But, we still might want to sort it to ensure (better) progress (no livelock). MutateRows sorts them (by using a TreeSet with Bytes.BYTES_COMPARATOR, for exactly this reason. Maybe this should be called out here, by making the argument a SortedSet. - Lars was (Author: jirapos...@reviews.apache.org): bq. On 2012-02-06 15:52:43, Amitanand Aiyer wrote: bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 4212 bq. https://reviews.apache.org/r/3748/diff/2/?file=72266#file72266line4212 bq. bq. Do we need to be sorting rowsToLock? bq. bq. I'm thinking of multiple concurrent mutateRows operation, trying to lock the same set of rows. bq. bq. Perhaps, throwing IOException is going to prevent us from a situation where we end up with a deadlock. But, we still might want to sort it to ensure (better) progress (no livelock). MutateRows sorts them (by using a TreeSet with Bytes.BYTES_COMPARATOR, for exactly this reason. Maybe this should be called out here, by making the argument a SortedSet. - Lars --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3748/#review4833 --- On 2012-02-03 19:59:55, Lars Hofhansl wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/3748/ bq. --- bq. bq. (Updated 2012-02-03 19:59:55) bq. bq. bq. Review request for hbase. bq. bq. bq. Summary bq. --- bq. bq. This builds on HBASE-3584, HBASE-5203, and HBASE-5304. bq. bq. Multiple Rows can be locked and applied atomically as long as the application ensures that all rows reside in the same Region (by presplitting or a custom RegionSplitPolicy). bq. At SFDC we can use this to colocate subsets of a tenant's data and allow atomic operations over these subsets. bq. bq. Obviously this is an advanced features and this prominently called out in the Javadoc. bq. bq. bq. This addresses bug HBASE-5229. bq. https://issues.apache.org/jira/browse/HBASE-5229 bq. bq. bq. Diffs bq. - bq. bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java 1239953 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java 1239953 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiRowMutation.java PRE-CREATION bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java 1239953 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java 1239953 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 1239953 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1239953 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1239953 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/rest/client/RemoteHTable.java 1239953 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java 1239953 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestAtomicOperation.java 1239953 bq. bq. Diff: https://reviews.apache.org/r/3748/diff bq. bq. bq. Testing bq. --- bq. bq. Tests added to TestFromClientSide and TestAtomicOperation bq. bq. bq. Thanks, bq. bq. Lars bq. bq. Provide basic building blocks for multi-row local transactions.
[jira] [Issue Comment Edited] (HBASE-5229) Provide basic building blocks for multi-row local transactions.
[ https://issues.apache.org/jira/browse/HBASE-5229?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13202996#comment-13202996 ] Lars Hofhansl edited comment on HBASE-5229 at 2/8/12 12:29 AM: --- bq. On 2012-02-07 23:49:19, Jesse Yates wrote: bq. http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/MultiRowMutationEndpoint.java, line 39 bq. https://reviews.apache.org/r/3748/diff/4/?file=72911#file72911line39 bq. bq. This could be good to put into client.coprocessor, rather than client (when you make the move over). bq. bq. Michael Stack wrote: bq. This is a good point. Heh, I am way ahead of you guys :) Suggested already on the jira. Will have a new patch soon, hopefully the last revision. I would like to keep the test in TestFromClientSide, though. - Lars was (Author: jirapos...@reviews.apache.org): bq. On 2012-02-07 23:49:19, Jesse Yates wrote: bq. http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/MultiRowMutationEndpoint.java, line 39 bq. https://reviews.apache.org/r/3748/diff/4/?file=72911#file72911line39 bq. bq. This could be good to put into client.coprocessor, rather than client (when you make the move over). bq. bq. Michael Stack wrote: bq. This is a good point. Heh, I am way ahead of you guys :) Suggested already on the jira. Will have a new patch soon, hopefully the last revision. I would like to keep the test in TestFromClientSide, though. - Lars --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3748/#review4886 --- On 2012-02-07 19:41:16, Lars Hofhansl wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/3748/ bq. --- bq. bq. (Updated 2012-02-07 19:41:16) bq. bq. bq. Review request for hbase. bq. bq. bq. Summary bq. --- bq. bq. This builds on HBASE-3584, HBASE-5203, and HBASE-5304. bq. bq. Multiple Rows can be locked and applied atomically as long as the application ensures that all rows reside in the same Region (by presplitting or a custom RegionSplitPolicy). bq. At SFDC we can use this to colocate subsets of a tenant's data and allow atomic operations over these subsets. bq. bq. Obviously this is an advanced features and this prominently called out in the Javadoc. bq. bq. bq. This addresses bug HBASE-5229. bq. https://issues.apache.org/jira/browse/HBASE-5229 bq. bq. bq. Diffs bq. - bq. bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1241350 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1241350 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/MultiRowMutationEndpoint.java PRE-CREATION bq. http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/MultiRowMutationProtocol.java PRE-CREATION bq. http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java 1241350 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestAtomicOperation.java 1241350 bq. bq. Diff: https://reviews.apache.org/r/3748/diff bq. bq. bq. Testing bq. --- bq. bq. Tests added to TestFromClientSide and TestAtomicOperation bq. bq. bq. Thanks, bq. bq. Lars bq. bq. Provide basic building blocks for multi-row local transactions. - Key: HBASE-5229 URL: https://issues.apache.org/jira/browse/HBASE-5229 Project: HBase Issue Type: New Feature Components: client, regionserver Reporter: Lars Hofhansl Assignee: Lars Hofhansl Fix For: 0.94.0 Attachments: 5229-endpoint.txt, 5229-multiRow-v2.txt, 5229-multiRow.txt, 5229-seekto-v2.txt, 5229-seekto.txt, 5229.txt In the final iteration, this issue provides a generalized, public mutateRowsWithLocks method on HRegion, that can be used by coprocessors to implement atomic operations efficiently. Coprocessors are already region aware, which makes this is a good pairing of APIs. This feature is by design not available to the client via the HTable API. It took a long time to arrive at this and I apologize for the public exposure of my (erratic in retrospect) thought
[jira] [Issue Comment Edited] (HBASE-5229) Provide basic building blocks for multi-row local transactions.
[ https://issues.apache.org/jira/browse/HBASE-5229?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13202993#comment-13202993 ] Lars Hofhansl edited comment on HBASE-5229 at 2/8/12 12:29 AM: --- bq. On 2012-02-07 23:49:19, Jesse Yates wrote: bq. http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/MultiRowMutationEndpoint.java, line 39 bq. https://reviews.apache.org/r/3748/diff/4/?file=72911#file72911line39 bq. bq. This could be good to put into client.coprocessor, rather than client (when you make the move over). This is a good point. - Michael was (Author: jirapos...@reviews.apache.org): bq. On 2012-02-07 23:49:19, Jesse Yates wrote: bq. http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/MultiRowMutationEndpoint.java, line 39 bq. https://reviews.apache.org/r/3748/diff/4/?file=72911#file72911line39 bq. bq. This could be good to put into client.coprocessor, rather than client (when you make the move over). This is a good point. - Michael --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3748/#review4886 --- On 2012-02-07 19:41:16, Lars Hofhansl wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/3748/ bq. --- bq. bq. (Updated 2012-02-07 19:41:16) bq. bq. bq. Review request for hbase. bq. bq. bq. Summary bq. --- bq. bq. This builds on HBASE-3584, HBASE-5203, and HBASE-5304. bq. bq. Multiple Rows can be locked and applied atomically as long as the application ensures that all rows reside in the same Region (by presplitting or a custom RegionSplitPolicy). bq. At SFDC we can use this to colocate subsets of a tenant's data and allow atomic operations over these subsets. bq. bq. Obviously this is an advanced features and this prominently called out in the Javadoc. bq. bq. bq. This addresses bug HBASE-5229. bq. https://issues.apache.org/jira/browse/HBASE-5229 bq. bq. bq. Diffs bq. - bq. bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1241350 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1241350 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/MultiRowMutationEndpoint.java PRE-CREATION bq. http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/MultiRowMutationProtocol.java PRE-CREATION bq. http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java 1241350 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestAtomicOperation.java 1241350 bq. bq. Diff: https://reviews.apache.org/r/3748/diff bq. bq. bq. Testing bq. --- bq. bq. Tests added to TestFromClientSide and TestAtomicOperation bq. bq. bq. Thanks, bq. bq. Lars bq. bq. Provide basic building blocks for multi-row local transactions. - Key: HBASE-5229 URL: https://issues.apache.org/jira/browse/HBASE-5229 Project: HBase Issue Type: New Feature Components: client, regionserver Reporter: Lars Hofhansl Assignee: Lars Hofhansl Fix For: 0.94.0 Attachments: 5229-endpoint.txt, 5229-multiRow-v2.txt, 5229-multiRow.txt, 5229-seekto-v2.txt, 5229-seekto.txt, 5229.txt In the final iteration, this issue provides a generalized, public mutateRowsWithLocks method on HRegion, that can be used by coprocessors to implement atomic operations efficiently. Coprocessors are already region aware, which makes this is a good pairing of APIs. This feature is by design not available to the client via the HTable API. It took a long time to arrive at this and I apologize for the public exposure of my (erratic in retrospect) thought processes. Was: HBase should provide basic building blocks for multi-row local transactions. Local means that we do this by co-locating the data. Global (cross region) transactions are not discussed here. After a bit of discussion two solutions have emerged: 1. Keep the row-key for determining grouping and location and allow efficient intra-row scanning. A client application would then model tables as HBase-rows. 2. Define a
[jira] [Issue Comment Edited] (HBASE-5229) Provide basic building blocks for multi-row local transactions.
[ https://issues.apache.org/jira/browse/HBASE-5229?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13203080#comment-13203080 ] Lars Hofhansl edited comment on HBASE-5229 at 2/8/12 1:13 AM: -- --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3748/#review4889 --- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/coprocessor/MultiRowMutationProtocol.java https://reviews.apache.org/r/3748/#comment10786 Replace operations with transactions. http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java https://reviews.apache.org/r/3748/#comment10787 Please plug in testMultiRowMutation http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java https://reviews.apache.org/r/3748/#comment10788 testMultiRowMutation here as well. - Ted was (Author: jirapos...@reviews.apache.org): --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3748/#review4889 --- http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/coprocessor/MultiRowMutationProtocol.java https://reviews.apache.org/r/3748/#comment10786 Replace operations with transactions. http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java https://reviews.apache.org/r/3748/#comment10787 Please plug in testMultiRowMutation http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java https://reviews.apache.org/r/3748/#comment10788 testMultiRowMutation here as well. - Ted On 2012-02-08 00:15:28, Lars Hofhansl wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/3748/ bq. --- bq. bq. (Updated 2012-02-08 00:15:28) bq. bq. bq. Review request for hbase. bq. bq. bq. Summary bq. --- bq. bq. This builds on HBASE-3584, HBASE-5203, and HBASE-5304. bq. bq. Multiple Rows can be locked and applied atomically as long as the application ensures that all rows reside in the same Region (by presplitting or a custom RegionSplitPolicy). bq. At SFDC we can use this to colocate subsets of a tenant's data and allow atomic operations over these subsets. bq. bq. Obviously this is an advanced features and this prominently called out in the Javadoc. bq. bq. bq. This addresses bug HBASE-5229. bq. https://issues.apache.org/jira/browse/HBASE-5229 bq. bq. bq. Diffs bq. - bq. bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/coprocessor/MultiRowMutationEndpoint.java PRE-CREATION bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/coprocessor/MultiRowMutationProtocol.java PRE-CREATION bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1241536 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1241536 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java 1241536 bq. http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestAtomicOperation.java 1241536 bq. bq. Diff: https://reviews.apache.org/r/3748/diff bq. bq. bq. Testing bq. --- bq. bq. Tests added to TestFromClientSide and TestAtomicOperation bq. bq. bq. Thanks, bq. bq. Lars bq. bq. Provide basic building blocks for multi-row local transactions. - Key: HBASE-5229 URL: https://issues.apache.org/jira/browse/HBASE-5229 Project: HBase Issue Type: New Feature Components: client, regionserver Reporter: Lars Hofhansl Assignee: Lars Hofhansl Fix For: 0.94.0 Attachments: 5229-endpoint.txt, 5229-multiRow-v2.txt, 5229-multiRow.txt, 5229-seekto-v2.txt, 5229-seekto.txt, 5229.txt In the final iteration, this issue provides a generalized, public mutateRowsWithLocks method on HRegion, that can be used by coprocessors to implement atomic operations efficiently. Coprocessors are already region aware, which makes this
