[
https://issues.apache.org/jira/browse/HBASE-5203?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13186993#comment-13186993
]
[email protected] commented on HBASE-5203:
------------------------------------------------------
bq. On 2012-01-16 15:27:14, Ted Yu wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java,
line 149
bq. > <https://reviews.apache.org/r/3510/diff/2/?file=68986#file68986line149>
bq. >
bq. > I think DoNotRetryIOException may be more appropriate here.
Sure. Although this is client side code, so there is no notion of retry. (Put
does the same)
bq. On 2012-01-16 15:27:14, Ted Yu wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java,
line 4170
bq. > <https://reviews.apache.org/r/3510/diff/2/?file=68987#file68987line4170>
bq. >
bq. > Should we allow caller to pass clusterId ?
bq. > That parameter would be used at line 4213.
The clusterID is only used for replication. Only plain Puts and Deletes need to
use an optional clusterId (when executed from the ReplicationSink). All other
operations do (and should) use the local clusterID.
bq. On 2012-01-16 15:27:14, Ted Yu wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java,
line 4184
bq. > <https://reviews.apache.org/r/3510/diff/2/?file=68987#file68987line4184>
bq. >
bq. > The original intent of this check being inside for loop was to
populate walEdits.
bq. > Now we can lift this check to after line 4157.
Correct. But I still need to execute and check all preHooks before the 1st
WALEdit is written.
bq. On 2012-01-16 15:27:14, Ted Yu wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java,
line 4214
bq. > <https://reviews.apache.org/r/3510/diff/2/?file=68987#file68987line4214>
bq. >
bq. > There is only one WALEdit now, right ?
Correct. Should read "and apply edits" (there are many edits in the one WALEdit)
bq. On 2012-01-16 15:27:14, Ted Yu wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java,
line 98
bq. > <https://reviews.apache.org/r/3510/diff/2/?file=68988#file68988line98>
bq. >
bq. > I think the original javadoc should be modified to indicate the
support of Put and Delete.
Agreed.
bq. On 2012-01-16 15:27:14, Ted Yu wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java,
line 175
bq. > <https://reviews.apache.org/r/3510/diff/2/?file=68988#file68988line175>
bq. >
bq. > I think 'to a map from key to values' may be clearer.
bq. > Otherwise people have to read the method body to fully understand.
I think this should be a static util method somewhere(?)
bq. On 2012-01-16 15:27:14, Ted Yu wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java,
line 202
bq. > <https://reviews.apache.org/r/3510/diff/2/?file=68988#file68988line202>
bq. >
bq. > I don't see InterruptedException declared to be thrown by this
method.
bq. > IE is caught at line 171.
Argghh... HTable.batch() throws it, and my first attempt was to pass it on.
This is a leftover will be remove.
Thanks for the keen eyes.
bq. On 2012-01-16 15:27:14, Ted Yu wrote:
bq. >
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java,
line 1788
bq. > <https://reviews.apache.org/r/3510/diff/2/?file=68987#file68987line1788>
bq. >
bq. > Please replace this parameter with clusterId.
I knew you would find some Javadoc I missed :)
Will fix.
- Lars
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3510/#review4394
-----------------------------------------------------------
On 2012-01-16 07:58:33, Lars Hofhansl wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/3510/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-01-16 07:58:33)
bq.
bq.
bq. Review request for hbase, Ted Yu and Michael Stack.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Basically a rewrite (sorry about that) of HBASE-3485 "Allow atomic
put/delete in one call".
bq. This makes this actually correct in the case of RegionServer failures
(HBASE-3485 was correct for all scenarios but RegionServer failures).
bq. HRegion.mutateRow(...) now groups all edits into a single WALEdit and
appends all edits in one call. Only then are the memstore edits applied.
bq. This is the first time that WALEdits can contain KVs from different types
of operations. So I also had to fix the replication code to understand that.
bq. WAL recovery already handles this case.
bq.
bq.
bq. This addresses bug HBASE-5203.
bq. https://issues.apache.org/jira/browse/HBASE-5203
bq.
bq.
bq. Diffs
bq. -----
bq.
bq.
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java
1231744
bq.
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
1231744
bq.
http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java
1231744
bq.
http://svn.apache.org/repos/asf/hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
1231744
bq.
bq. Diff: https://reviews.apache.org/r/3510/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. * Tests added in HBASE-3485
bq. * manual testing.
bq. * getting a full test run right now
bq.
bq.
bq. Thanks,
bq.
bq. Lars
bq.
bq.
> Group atomic put/delete operation into a single WALEdit to handle region
> server failures.
> -----------------------------------------------------------------------------------------
>
> Key: HBASE-5203
> URL: https://issues.apache.org/jira/browse/HBASE-5203
> Project: HBase
> Issue Type: Sub-task
> Components: client, coprocessors, regionserver
> Reporter: Lars Hofhansl
> Assignee: Lars Hofhansl
> Fix For: 0.94.0
>
> Attachments: 5203.txt
>
>
> HBASE-3584 does not not provide fully atomic operation in case of region
> server failures (see explanation there).
> What should happen is that either (1) all edits are applied via a single
> WALEdit, or (2) the WALEdits are applied in async mode and then sync'ed
> together.
> For #1 it is not clear whether it is advisable to manage multiple *different*
> operations (Put/Delete) via a single WAL edit. A quick check reveals that WAL
> replay on region startup would work, but that replication would need to be
> adapted. The refactoring needed would be non-trivial.
> #2 Might actually not work, as another operation could request sync'ing a
> later edit and hence flush these entries out as well.
--
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
