w.r.t. MasterObserver.postCreateTable(), does it make sense to provide this hook at the end of CreateTableHandler.process() now that HBASE-3229 has been committed ?
Cheers On Fri, Aug 26, 2011 at 2:57 PM, Gary Helmling <[email protected]> wrote: > Hi Ming, > > Sorry, gmail flagged this as spam for some reason, so I didn't see it until > now. > > 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. > > > Gary >
