You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by Ryan Rawson <ry...@gmail.com> on 2010/11/05 09:43:23 UTC
Review Request: HBASE-2989 MultiPut makes proper error handling impossible
and leads to corrupted data
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1176/
-----------------------------------------------------------
Review request for hbase.
Summary
-------
This is a change to batch() that substantially increases the error handling. Included is a new Exception type with a lot more details of what failed and where. Users will be able to also query the thrown exception to see if there is a possible cluster problem. They will be able to log the server addresses that had issues.
The TestMultiParallel test case passes, I need to run the rest.
This addresses bug HBASE-2898.
http://issues.apache.org/jira/browse/HBASE-2898
Diffs
-----
trunk/pom.xml 1031470
trunk/src/main/java/org/apache/hadoop/hbase/client/HConnection.java 1031470
trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 1031470
trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java 1031470
trunk/src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java 1031470
trunk/src/main/java/org/apache/hadoop/hbase/client/MultiResponse.java 1031470
trunk/src/main/java/org/apache/hadoop/hbase/client/RetriesExhaustedWithDetailsException.java PRE-CREATION
trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1031470
trunk/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerStoppedException.java 1031470
trunk/src/main/java/org/apache/hadoop/hbase/rest/client/RemoteHTable.java 1031470
trunk/src/test/java/org/apache/hadoop/hbase/client/MultiResponseTest.java PRE-CREATION
trunk/src/test/java/org/apache/hadoop/hbase/client/TestMultiParallel.java 1031470
Diff: http://review.cloudera.org/r/1176/diff
Testing
-------
Thanks,
Ryan
Re: Review Request: HBASE-2989 MultiPut makes proper error handling
impossible and leads to corrupted data
Posted by Ryan Rawson <ry...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1176/#review1875
-----------------------------------------------------------
trunk/src/main/java/org/apache/hadoop/hbase/client/HConnection.java
<http://review.cloudera.org/r/1176/#comment6075>
this was called by HTable, but those have shifted to use processBatch() instead.
- Ryan
On 2010-11-05 01:43:22, Ryan Rawson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/1176/
> -----------------------------------------------------------
>
> (Updated 2010-11-05 01:43:22)
>
>
> Review request for hbase.
>
>
> Summary
> -------
>
> This is a change to batch() that substantially increases the error handling. Included is a new Exception type with a lot more details of what failed and where. Users will be able to also query the thrown exception to see if there is a possible cluster problem. They will be able to log the server addresses that had issues.
>
> The TestMultiParallel test case passes, I need to run the rest.
>
>
> This addresses bug HBASE-2898.
> http://issues.apache.org/jira/browse/HBASE-2898
>
>
> Diffs
> -----
>
> trunk/pom.xml 1031470
> trunk/src/main/java/org/apache/hadoop/hbase/client/HConnection.java 1031470
> trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 1031470
> trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java 1031470
> trunk/src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java 1031470
> trunk/src/main/java/org/apache/hadoop/hbase/client/MultiResponse.java 1031470
> trunk/src/main/java/org/apache/hadoop/hbase/client/RetriesExhaustedWithDetailsException.java PRE-CREATION
> trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1031470
> trunk/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerStoppedException.java 1031470
> trunk/src/main/java/org/apache/hadoop/hbase/rest/client/RemoteHTable.java 1031470
> trunk/src/test/java/org/apache/hadoop/hbase/client/MultiResponseTest.java PRE-CREATION
> trunk/src/test/java/org/apache/hadoop/hbase/client/TestMultiParallel.java 1031470
>
> Diff: http://review.cloudera.org/r/1176/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ryan
>
>
Re: Review Request: HBASE-2989 MultiPut makes proper error handling
impossible and leads to corrupted data
Posted by Ryan Rawson <ry...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1176/#review1855
-----------------------------------------------------------
trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
<http://review.cloudera.org/r/1176/#comment6033>
people can selective not catch IE and then let their regular abort do it's job.
trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java
<http://review.cloudera.org/r/1176/#comment6034>
yes, absolutely, because at the end we check every single entry in the results[] and if there are any failures (null, Throwable instances) we throw an exception. So one can be sure that everything is a Result during success, and if you get an exception you have the Throwable handy dandy in both the results array and in thrown exception.
trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java
<http://review.cloudera.org/r/1176/#comment6035>
this is a brand new method, never was in any previous 0.89 DR even.
trunk/src/main/java/org/apache/hadoop/hbase/client/MultiResponse.java
<http://review.cloudera.org/r/1176/#comment6036>
it could be either a Throwable or a Result. batch() shouldn't throw many exceptions, they should be identified and lined up with each index entry.
trunk/src/test/java/org/apache/hadoop/hbase/client/MultiResponseTest.java
<http://review.cloudera.org/r/1176/#comment6037>
I blame SVN! It was including my scratch work :-)
- Ryan
On 2010-11-05 01:43:22, Ryan Rawson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/1176/
> -----------------------------------------------------------
>
> (Updated 2010-11-05 01:43:22)
>
>
> Review request for hbase.
>
>
> Summary
> -------
>
> This is a change to batch() that substantially increases the error handling. Included is a new Exception type with a lot more details of what failed and where. Users will be able to also query the thrown exception to see if there is a possible cluster problem. They will be able to log the server addresses that had issues.
>
> The TestMultiParallel test case passes, I need to run the rest.
>
>
> This addresses bug HBASE-2898.
> http://issues.apache.org/jira/browse/HBASE-2898
>
>
> Diffs
> -----
>
> trunk/pom.xml 1031470
> trunk/src/main/java/org/apache/hadoop/hbase/client/HConnection.java 1031470
> trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 1031470
> trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java 1031470
> trunk/src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java 1031470
> trunk/src/main/java/org/apache/hadoop/hbase/client/MultiResponse.java 1031470
> trunk/src/main/java/org/apache/hadoop/hbase/client/RetriesExhaustedWithDetailsException.java PRE-CREATION
> trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1031470
> trunk/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerStoppedException.java 1031470
> trunk/src/main/java/org/apache/hadoop/hbase/rest/client/RemoteHTable.java 1031470
> trunk/src/test/java/org/apache/hadoop/hbase/client/MultiResponseTest.java PRE-CREATION
> trunk/src/test/java/org/apache/hadoop/hbase/client/TestMultiParallel.java 1031470
>
> Diff: http://review.cloudera.org/r/1176/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ryan
>
>
Re: Review Request: HBASE-2989 MultiPut makes proper error handling
impossible and leads to corrupted data
Posted by st...@duboce.net.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1176/#review1852
-----------------------------------------------------------
Patch looks great. Needs the test filled out before +1 and do all unit tests pass? It changes a bunch of critical code so wouldn't be surprised if unexpected side effects.
trunk/pom.xml
<http://review.cloudera.org/r/1176/#comment6013>
You didn't mean this, right? We can't have direct cloudera dependency.
trunk/src/main/java/org/apache/hadoop/hbase/client/HConnection.java
<http://review.cloudera.org/r/1176/#comment6014>
No one depends on these?
trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
<http://review.cloudera.org/r/1176/#comment6015>
review board is showing a bunch of white space on these lines? Tabs?
trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
<http://review.cloudera.org/r/1176/#comment6016>
You changing public API here?
I suppose we are but only in HCM which is rarely used by other than internals... disregard this comment since I see you convert it in HTable below to an IOE.
trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
<http://review.cloudera.org/r/1176/#comment6018>
Hmmm... would make life easier if we just let out the IE.
trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
<http://review.cloudera.org/r/1176/#comment6020>
Long line.
trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
<http://review.cloudera.org/r/1176/#comment6023>
Yeah, cleaner if IE is let out (I suppose).
trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java
<http://review.cloudera.org/r/1176/#comment6024>
This is kinda ugly. For sure exceptions have been purged here?
trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java
<http://review.cloudera.org/r/1176/#comment6025>
Adding an IE here is ok? Is this a new method in 0.90?
trunk/src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java
<http://review.cloudera.org/r/1176/#comment6026>
Oh, ok... then IE is fine.
trunk/src/main/java/org/apache/hadoop/hbase/client/MultiResponse.java
<http://review.cloudera.org/r/1176/#comment6027>
Could there be exception here or its always Result? If always Result why change type to Object?
trunk/src/main/java/org/apache/hadoop/hbase/client/RetriesExhaustedWithDetailsException.java
<http://review.cloudera.org/r/1176/#comment6028>
Missing class comment on what this doohickey does
trunk/src/test/java/org/apache/hadoop/hbase/client/MultiResponseTest.java
<http://review.cloudera.org/r/1176/#comment6029>
Hey man, great test! Can you add something in here? Smile.
- stack
On 2010-11-05 01:43:22, Ryan Rawson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/1176/
> -----------------------------------------------------------
>
> (Updated 2010-11-05 01:43:22)
>
>
> Review request for hbase.
>
>
> Summary
> -------
>
> This is a change to batch() that substantially increases the error handling. Included is a new Exception type with a lot more details of what failed and where. Users will be able to also query the thrown exception to see if there is a possible cluster problem. They will be able to log the server addresses that had issues.
>
> The TestMultiParallel test case passes, I need to run the rest.
>
>
> This addresses bug HBASE-2898.
> http://issues.apache.org/jira/browse/HBASE-2898
>
>
> Diffs
> -----
>
> trunk/pom.xml 1031470
> trunk/src/main/java/org/apache/hadoop/hbase/client/HConnection.java 1031470
> trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 1031470
> trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java 1031470
> trunk/src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java 1031470
> trunk/src/main/java/org/apache/hadoop/hbase/client/MultiResponse.java 1031470
> trunk/src/main/java/org/apache/hadoop/hbase/client/RetriesExhaustedWithDetailsException.java PRE-CREATION
> trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1031470
> trunk/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerStoppedException.java 1031470
> trunk/src/main/java/org/apache/hadoop/hbase/rest/client/RemoteHTable.java 1031470
> trunk/src/test/java/org/apache/hadoop/hbase/client/MultiResponseTest.java PRE-CREATION
> trunk/src/test/java/org/apache/hadoop/hbase/client/TestMultiParallel.java 1031470
>
> Diff: http://review.cloudera.org/r/1176/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ryan
>
>