[
https://issues.apache.org/jira/browse/HBASE-4014?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13071514#comment-13071514
]
[email protected] commented on HBASE-4014:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/969/#review1198
-----------------------------------------------------------
Some comments below Eugene. This thing looks useful and almost done. Lets get
it in!
src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorEnvironment.java
<https://reviews.apache.org/r/969/#comment2606>
Do we need to add this? Doesn't every object inherit Object and so have a
toString?
src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java
<https://reviews.apache.org/r/969/#comment2607>
Do you think this needed Eugene? Is coprocessors a List? What if you
toString'd it? Maybe'll do right thing (with square bracket delimiters rather
than curly's but that might be ok)
src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java
<https://reviews.apache.org/r/969/#comment2608>
Whats masterServices? I think it subclasses Server? If you do
getServerName or something, that'll give you something better than 'master'.
It'll include port and startcode which could be important debugging (more
important for the RS case than for Master but could be important if
multimasters).
src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java
<https://reviews.apache.org/r/969/#comment2610>
Nice comment
src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java
<https://reviews.apache.org/r/969/#comment2609>
Abort seems radical
src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java
<https://reviews.apache.org/r/969/#comment2611>
The convention is to put } catch { on the one line rather than line break
after the } (no biggie)
src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
<https://reviews.apache.org/r/969/#comment2612>
Good
src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java
<https://reviews.apache.org/r/969/#comment2613>
We are importing but we don't seem to use the imports, is that so?
src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
<https://reviews.apache.org/r/969/#comment2614>
Is there any diff in the above changes?
src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java
<https://reviews.apache.org/r/969/#comment2615>
Same here
src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterCoprocessorException.java
<https://reviews.apache.org/r/969/#comment2616>
No need for these extra new lines
src/test/java/org/apache/hadoop/hbase/master/TestActiveMasterManager.java
<https://reviews.apache.org/r/969/#comment2617>
Unused import?
src/test/java/org/apache/hadoop/hbase/master/TestActiveMasterManager.java
<https://reviews.apache.org/r/969/#comment2618>
Added line we don't need?
src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java
<https://reviews.apache.org/r/969/#comment2619>
Unused import
src/test/java/org/apache/hadoop/hbase/master/TestClockSkewDetection.java
<https://reviews.apache.org/r/969/#comment2620>
Unused import?
src/test/java/org/apache/hadoop/hbase/master/TestLogsCleaner.java
<https://reviews.apache.org/r/969/#comment2621>
Unused import?
src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java
<https://reviews.apache.org/r/969/#comment2622>
Unused import?
src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java
<https://reviews.apache.org/r/969/#comment2623>
Addition of commented out code
src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java
<https://reviews.apache.org/r/969/#comment2624>
Unused import
- Michael
On 2011-07-14 23:39:07, 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-07-14 23:39:07)
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/master/MasterCoprocessorHost.java
aa930f5
bq.
src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorEnvironment.java
54ccd6f
bq. src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java
18ba6e7
bq.
src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
c2b3558
bq.
src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java
8ffa086
bq. src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
4fa82c0
bq. src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java 6af0ecf
bq.
src/test/java/org/apache/hadoop/hbase/coprocessor/BuggyRegionObserver.java
PRE-CREATION
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.
src/test/java/org/apache/hadoop/hbase/master/TestActiveMasterManager.java
7f19c72
bq. src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java
78e7d62
bq. src/test/java/org/apache/hadoop/hbase/master/TestClockSkewDetection.java
c571227
bq. src/test/java/org/apache/hadoop/hbase/master/TestLogsCleaner.java
fc05e47
bq.
src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java
aa48c22
bq.
src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java
dd5dc3e
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
>
>
> 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
