Sorry, gmail flagged this as spam for some reason, so I didn't see it until
On Wed, Aug 24, 2011 at 12:21 AM, Ma, Ming <[EMAIL PROTECTED]> wrote:
> As part of fixing 3229, I have some questions about usage scenarios of
> coprocessor in table operations, e.g., MasterObserver::preEnableTable, etc.
> 1. Operations like preAddColumn, etc. don't honor coprocessor's
> request to bypass default behavior, as in MasterCoProcessorHost.java. Is
> that a bug or there is some reason for that? It seems useful feature if
> coprocessor can disallow addColumn, for example.
This seems like an oversight that we should fix. Taking a quick look over
MasterCoprocessorHost, I would agree that all of the preXXX hooks for
create, modify, delete of tables should support bypass. Early on in
implementing coprocessors, there was some question of to what extent they
should be able to "break" core HBase functionality. So I might have left
bypass handling out of these methods out of an over-abundance of caution,
but if so it seems unwarranted. The only hooks that I would say should
_not_ support "bypass" are preShutdown() and preStopMaster().
> 2. Some of these operations like EnableTable is async. postEnableTable
> is called right after the request is queued to the thread pool. So there is
> no guarantee the process is done by the time postEnableTable is called. Yes,
> postEnableTable won't be called if the validation in the constructor of
> EnableTableHandler throws exception. Still, what are the scenarios that
> postEnableTable can be useful in the async implementation?
Sounds like we should clearly note this in the javadoc for
MasterObserver.postEnableTable(). I don't think the async handling itself
renders postEnableTable not useful, though. In general I find the postXXX
hooks most useful for listener-type operations. As long as implementors are
aware of the timing, I don't think it's necessarily an issue.
> 3. Method signature in MasterObserver::preCreateTable uses
> tableDescriptor+splitKey; MasterObserver:: postCreateTable signature uses
> HRegionInfo. They are essential the same thing. Why can't we use the same
> signature, like HRegionInfo instead?
We might have used that signature for preCreateTable() to mirror the RPC
call from clients? But changing this to use HRegionInfo would allow us to
also change the return type to HRegionInfo, which would allow implementors
to override the region splits. This seems like an additional bonus.
All of these sound like good and useful changes. You want to open up a jira
Ming? Feel free to work up a patch as well! I'd be happy to review.