[ https://issues.apache.org/jira/browse/HBASE-19953?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16370671#comment-16370671 ]
Josh Elser commented on HBASE-19953: ------------------------------------ {quote}What you have done here is clean. Maybe some more comment to the effect that the latch is just-for the Procedure preparation – that we are not blocking for the whole procedure run – though the naming of the latch is enough I suppose. That noop latch is 'interesting'... {quote} Haha, yes, I agree with you. I wanted to leave a comment here also covering the fact that we _could_ do this differently if holding open this RPC is a problem. Let me try to document here as well for the inevitable case when I forget... Taking {{createTable}} as an example, the client submits the async calls which submits the procedure to create a table. With this patch, the Master will block the client from receiving the procedure ID it needs to wait on for the createTable to finish to also ensure that the prepare finishes (sanity-checks – the table we want to create doesn't exist already). This wait _should_ be short so this is OK. The worry would be that (for an unseen reason), this takes a long time or gets stuck, which leaves an RPC+thread open in the Master, leading to a denial of service. Instead, we could make two "async checks" on the client: the first would be to block for the result of the "prepare" of our createTable, and then a second to ensure that the procedure actually completes. This is obviously more work, but it's something we can keep in our back-pocket. Will add some docs to the commit and push this one. Thanks for the review, Stack! > Avoid calling post* hook when procedure fails > --------------------------------------------- > > Key: HBASE-19953 > URL: https://issues.apache.org/jira/browse/HBASE-19953 > Project: HBase > Issue Type: Bug > Components: master, proc-v2 > Reporter: Ramesh Mani > Assignee: Josh Elser > Priority: Critical > Fix For: 2.0.0-beta-2 > > Attachments: HBASE-19952.001.branch-2.patch, > HBASE-19953.002.branch-2.patch, HBASE-19953.003.branch-2.patch > > > Ramesh pointed out a case where I think we're mishandling some post\* > MasterObserver hooks. Specifically, I'm looking at the deleteNamespace. > We synchronously execute the DeleteNamespace procedure. When the user > provides a namespace that isn't empty, the procedure does a rollback (which > is just a no-op), but this doesn't propagate an exception up to the > NonceProcedureRunnable in {{HMaster#deleteNamespace}}. It took Ramesh > pointing it out a bit better to me that the code executes a bit differently > than we actually expect. > I think we need to double-check our post hooks and make sure we aren't > invoking them when the procedure actually failed. cc/ [~Apache9], [~stack]. -- This message was sent by Atlassian JIRA (v7.6.3#76005)