[
https://issues.apache.org/jira/browse/HBASE-4014?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13109087#comment-13109087
]
jirapos...@reviews.apache.org commented on HBASE-4014:
------------------------------------------------------
bq. On 2011-09-08 23:46:17, Gary Helmling wrote:
bq. > src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java,
line 638
bq. > <https://reviews.apache.org/r/969/diff/9/?file=38128#file38128line638>
bq. >
bq. > This should default to false.
Fixed; please see latest patch.
bq. On 2011-09-08 23:46:17, Gary Helmling wrote:
bq. >
src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterCoprocessorException.java,
line 67
bq. > <https://reviews.apache.org/r/969/diff/9/?file=38134#file38134line67>
bq. >
bq. > Does this need to be a separate thread? Can the contents of the
run() method just be inline in testExceptionFromCoprocessorWhenCreatingTable()?
In my testing, if the server (master or regionserver) aborts, it seems like the
client becomes unresponsive and the test times out and fails. However, if I
create a separate thread, the main thread can terminate properly and the test
passes.
I removed the separate Threads for the two tests where an abort is not expected
(TestMasterCoprocessorExceptionWithRemove.java and
TestRegionServerExceptionWithRemove.java).
bq. On 2011-09-08 23:46:17, Gary Helmling wrote:
bq. >
src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterCoprocessorException.java,
line 156
bq. > <https://reviews.apache.org/r/969/diff/9/?file=38134#file38134line156>
bq. >
bq. > Do we need this test? If we're already doing the same tests in
TestMasterObserver, it doesn't seem like it. Has anything been added to this
method that we need?
You are right; removed.
bq. On 2011-09-08 23:46:17, Gary Helmling wrote:
bq. >
src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionServerCoprocessorException.java,
line 89
bq. > <https://reviews.apache.org/r/969/diff/9/?file=38135#file38135line89>
bq. >
bq. > Name should be something like testExceptionDuringPut?
Renamed; thanks.
- Eugene
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/969/#review1805
-----------------------------------------------------------
On 2011-09-06 19:08:59, Eugene Koontz wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/969/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-09-06 19:08:59)
bq.
bq.
bq. Review request for hbase, Gary Helmling and Mingjie Lai.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. https://issues.apache.org/jira/browse/HBASE-4014 Coprocessors: Flag the
presence of coprocessors in logged exceptions
bq.
bq. The general gist here is to wrap each of
{Master,RegionServer}CoprocessorHost's coprocessor call inside a
bq.
bq. "try { ... } catch (Throwable e) { handleCoprocessorThrowable(e) }"
bq.
bq. block.
bq.
bq. handleCoprocessorThrowable() is responsible for either passing 'e' along
to the client (if 'e' is an IOException) or, otherwise, aborting the service
(Regionserver or Master).
bq.
bq. The abort message contains a list of the loaded coprocessors for crash
analysis.
bq.
bq.
bq. This addresses bug HBASE-4014.
bq. https://issues.apache.org/jira/browse/HBASE-4014
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java
4e492e1
bq. src/main/java/org/apache/hadoop/hbase/master/HMaster.java 3f60653
bq. src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java
aa930f5
bq. src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
8ff6e62
bq.
src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
5796413
bq. src/main/resources/hbase-default.xml 2c8f44b
bq.
src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterCoprocessorException.java
PRE-CREATION
bq.
src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionServerCoprocessorException.java
PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/969/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. patch includes two tests:
bq.
bq. TestMasterCoprocessorException.java
bq. TestRegionServerCoprocessorException.java
bq.
bq. both tests pass in my build environment.
bq.
bq.
bq. Thanks,
bq.
bq. Eugene
bq.
bq.
> Coprocessors: Flag the presence of coprocessors in logged exceptions
> --------------------------------------------------------------------
>
> Key: HBASE-4014
> URL: https://issues.apache.org/jira/browse/HBASE-4014
> Project: HBase
> Issue Type: Improvement
> Components: coprocessors
> Reporter: Andrew Purtell
> Assignee: Eugene Koontz
> Fix For: 0.92.0
>
> Attachments: HBASE-4014.patch, HBASE-4014.patch, HBASE-4014.patch,
> HBASE-4014.patch, HBASE-4014.patch
>
>
> For some initial triage of bug reports for core versus for deployments with
> loaded coprocessors, we need something like the Linux kernel's taint flag,
> and list of linked in modules that show up in the output of every OOPS, to
> appear above or below exceptions that appear in the logs.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
