[jira] [Issue Comment Edited] (HBASE-5584) Coprocessor hooks can be called in the respective handlers

["Andrew Purtell (JIRA)" <ji...@apache.org>]

    [ 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