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

Andrew Purtell edited comment on HBASE-5584 at 5/4/12 1:37 AM:
---------------------------------------------------------------

bq. Please consider making a note in the javadoc of MasterObserver which hooks 
are called synchronously with respect to RPC actions and which are not. 

This patch goes beyond such notation and distinguishes two classes of 
coprocessor hook now, 1) hooks called inline with RPC processing which can 
bypass(), and 2) a new class of hooks called from async handler code that can 
note some action but not override it. 

Shouldn't we still allow overriding default behavior? Like with compaction. 
There we tell the user that calling bypass() without providing some kind of 
alternate writer will drop all data on the floor. Likewise, if a hook in 
EnableTableHandler doesn't actually enable the table and calls bypass(), then 
we can expect this was as intended, but clearly state this in the Javadoc.

Edit: Sorry, not clear, I am +0 on the patch itself as long as we're all ok 
with the above. There is some newly added commented out code in the patch e.g.:

{code}
+    /*while (!admin.isTableEnabled(htd.getName())
+        && !admin.isTableAvailable(htd.getName())) {
+      Thread.sleep(50);
+    }*/
{code}

Remove that on commit and I'm +1.
                
      was (Author: apurtell):
    bq. Please consider making a note in the javadoc of MasterObserver which 
hooks are called synchronously with respect to RPC actions and which are not. 

This patch goes beyond such notation and distinguishes two classes of 
coprocessor hook now, 1) hooks called inline with RPC processing which can 
bypass(), and 2) a new class of hooks called from async handler code that can 
note some action but not override it. 

Shouldn't we still allow overriding default behavior? Like with compaction. 
There we tell the user that calling bypass() without providing some kind of 
alternate writer will drop all data on the floor. Likewise, if a hook in 
EnableTableHandler doesn't actually enable the table and calls bypass(), then 
we can expect this was as intended, but clearly state this in the Javadoc.




                  
> Coprocessor hooks can be called in the respective handlers
> ----------------------------------------------------------
>
>                 Key: HBASE-5584
>                 URL: https://issues.apache.org/jira/browse/HBASE-5584
>             Project: HBase
>          Issue Type: Improvement
>          Components: coprocessors
>            Reporter: ramkrishna.s.vasudevan
>            Assignee: ramkrishna.s.vasudevan
>             Fix For: 0.96.0
>
>         Attachments: HBASE-5584-1.patch, HBASE-5584-2.patch, HBASE-5584.patch
>
>
> Following points can be changed w.r.t to coprocessors
> -> Call preCreate, postCreate, preEnable, postEnable, etc. in their 
> respective handlers
> -> Currently it is called in the HMaster thus making the postApis async w.r.t 
> the handlers
> -> Similar is the case with the balancer.
> with current behaviour once we are in the postEnable(for eg) we any way need 
> to wait for the main enable handler to 
> be completed.
> We should ensure that we dont wait in the main thread so again we need to 
> spawn a thread and wait on that.
> On the other hand if the pre and post api is called on the handlers then only 
> that handler thread will be
> used in the pre/post apis
> If the above said plan is ok i can prepare a patch for all such related 
> changes.

--
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

        

Reply via email to