[ https://issues.apache.org/jira/browse/HBASE-5229?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13199518#comment-13199518 ]
jirapos...@reviews.apache.org commented on HBASE-5229: ------------------------------------------------------ bq. On 2012-02-03 04:45:54, Ted Yu wrote: bq. > The for the thourough review as usual. I'll have a new patch tomorrow. bq. On 2012-02-03 04:45:54, Ted Yu wrote: bq. > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiRowMutation.java, line 60 bq. > <https://reviews.apache.org/r/3748/diff/1/?file=72039#file72039line60> bq. > bq. > Can the two add() methods be combined into one which accepts Mutation ? That is because there are other mutations that I do not support (Increment/Append). bq. On 2012-02-03 04:45:54, Ted Yu wrote: bq. > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiRowMutation.java, line 65 bq. > <https://reviews.apache.org/r/3748/diff/1/?file=72039#file72039line65> bq. > bq. > This comment can be removed, right ? Yes bq. On 2012-02-03 04:45:54, Ted Yu wrote: bq. > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/RowMutation.java, line 39 bq. > <https://reviews.apache.org/r/3748/diff/1/?file=72040#file72040line39> bq. > bq. > From its name, RowMutation seems to refer to single row. It is a little confusing RowMutation extends MultiRowMutation. Yeah. Maybe I'll have a common super class instead. bq. On 2012-02-03 04:45:54, Ted Yu wrote: bq. > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/RowMutation.java, line 96 bq. > <https://reviews.apache.org/r/3748/diff/1/?file=72040#file72040line96> bq. > bq. > version would be read / written twice, right ? Yes. Need to fix that. bq. On 2012-02-03 04:45:54, Ted Yu wrote: bq. > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 4211 bq. > <https://reviews.apache.org/r/3748/diff/1/?file=72044#file72044line4211> bq. > bq. > rowsToLock.size() could be smaller than mutations.size(), right ? Oh yes... Good point. bq. On 2012-02-03 04:45:54, Ted Yu 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. > Can we refer regionName from rm (the MultiRowMutation) ? Yes, because all rows on the MultiRowMutation need to be in the same region. HTable just uses the first Mutation to find the region to the used. bq. On 2012-02-03 04:45:54, Ted Yu 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. > Is this method thread-safe ? Should be. Only uses local state or protected data structures (like put, get, etc) - Lars ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3748/#review4788 ----------------------------------------------------------- On 2012-02-03 01:29:58, 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 01:29:58) 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/client/RowMutation.java 1239953 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. > Explore 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-multiRow.txt, 5229-seekto-v2.txt, 5229-seekto.txt, > 5229.txt > > > 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 prefix-length in HTableDescriptor that defines a grouping of > rows. Regions will then never be split inside a grouping prefix. > #1 is true to the current storage paradigm of HBase. > #2 is true to the current client side API. > I will explore these two with sample patches here. > -------------------- > Was: > As discussed (at length) on the dev mailing list with the HBASE-3584 and > HBASE-5203 committed, supporting atomic cross row transactions within a > region becomes simple. > I am aware of the hesitation about the usefulness of this feature, but we > have to start somewhere. > Let's use this jira for discussion, I'll attach a patch (with tests) > momentarily to make this concrete. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira