[ 
https://issues.apache.org/jira/browse/HBASE-4014?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13099208#comment-13099208
 ] 

[email protected] commented on HBASE-4014:
------------------------------------------------------



bq.  On 2011-08-02 05:38:49, Gary Helmling wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java, 
line 578
bq.  > <https://reviews.apache.org/r/969/diff/3/?file=29401#file29401line578>
bq.  >
bq.  >     I saw in previous comments that this is needed because 
SortedCopyOnWriteSet doesn't implement toString().  So why not make 
SortedCopyOnWriteSet implement toString()?  Seems cleaner to me and more 
generic/reusable.
bq.  >     
bq.  >     Wherever this is implemented, use a StringBuilder to create the 
string to return, not repeated string concatenation.

I ended up removing the toString() implementation from SortedCopyOnWriteSet 
since I am simply storing the coprocessor names (not the whole 
CoprocessorEnvironment). Thank you for the StringBuilder suggestion, though.  


bq.  On 2011-08-02 05:38:49, Gary Helmling wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java, 
line 594
bq.  > <https://reviews.apache.org/r/969/diff/3/?file=29401#file29401line594>
bq.  >
bq.  >     Awfully long name, maybe just abortServer()?

Shortened to abortServer(); thanks for this.


bq.  On 2011-08-02 05:38:49, Gary Helmling wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java, 
line 80
bq.  > <https://reviews.apache.org/r/969/diff/3/?file=29402#file29402line80>
bq.  >
bq.  >     This and the following method names are awfully long.  It's just 
personal preference, but I like to keep things shorter.

Shortened to abortServer() also; thanks.


bq.  On 2011-08-02 05:38:49, Gary Helmling wrote:
bq.  > 
src/test/java/org/apache/hadoop/hbase/coprocessor/BuggyRegionObserver.java, 
line 27
bq.  > <https://reviews.apache.org/r/969/diff/3/?file=29404#file29404line27>
bq.  >
bq.  >     Minor nit, but why have this as a separate top-level class?  Seems 
like it could just be an inner class in TestRegionServerCoprocessorException 
same way that the BuggyMasterObserver is an inner class in 
TestMasterCoprocessorException.

Moved to interior of TestRegionServerCoprocessorException.java, thanks.


- Eugene


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/969/#review1261
-----------------------------------------------------------


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

        

Reply via email to