hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Phabricator (Commented) (JIRA)" <>
Subject [jira] [Commented] (HIVE-2720) Merge MetaStoreListener and HiveMetaHook interfaces
Date Fri, 20 Jan 2012 00:55:46 GMT


Phabricator commented on HIVE-2720:

cwsteinbach has requested changes to the revision "HIVE-2720 [jira] Merge MetaStoreListener
and HiveMetaHook interfaces".

  This also needs some test coverage. In particular I'd like to see testcases that cover the
scenario where multiple hooks are registered.

  metastore/src/java/org/apache/hadoop/hive/metastore/ Right now pre-methods
can also modify the input parameters before they are seen by the metastore. I don't think
this should be allowed. We can prevent it by only passing the pre-methods copies of the input
parameters, but that may be expensive. At the very least I think the javadoc needs to stipulate
that pre-methods are *not* allowed to mutate the input parameters.
  metastore/src/java/org/apache/hadoop/hive/metastore/ "and for some of
the of the operations"
  metastore/src/java/org/apache/hadoop/hive/metastore/ If multiple metahooks
are registered, then it's also possible that the first metahook will raise an exception and
the later metahooks won't see the event at all. This seems pretty bad from the standpoint
of a person trying to write a audit plugin, since they risk not seeing a lot of operations
that are failed by hooks that precede them in the list.

  Also, what happens if the second prehook throws an exception? Will the first posthook still
get called with a failure? It's possible that the first prehook created state that will get
cleared by the posthook, but this won't happen if the posthook is not called.

  I think the javadoc either needs to say that you (a) can't assume a posthook will get called
just because the prehook was called, or else (b) say that the posthook will always get called
if the prehook is called. Option (b) will make it much easier to write hooks at the expense
of more complexity on the Hive side.
  metastore/src/java/org/apache/hadoop/hive/metastore/ What happens if
a post-method throws an exception on an operation that has already been committed to the metastore?
What does that exception mean, and how is Hive supposed to handle it?
  metastore/src/java/org/apache/hadoop/hive/metastore/ Where's the prehook?
I noticed missing prehooks elsewhere as well.
  metastore/src/java/org/apache/hadoop/hive/metastore/ This is a hack
and is going to be broken by someone who doesn't understand that you're only calling get_database
to trigger the hook.

  Also, the fact that pre/post hooks aren't symmetrical is going to cause a lot of grief for
hook implementors. This needs to be fixed.

  This also introduces a time-of-check-to-time-of-use bug since you're not grabbing the DB
and table definitions from within the same txn.
  metastore/src/java/org/apache/hadoop/hive/metastore/ Is this a separate
bug? Is it being tracked in a different JIRA?
  metastore/src/java/org/apache/hadoop/hive/metastore/ This convention
for triggering the hooks is not acceptable. I think code analysis tools will probably flag
this since the db and tbl variables are not read.
  metastore/src/java/org/apache/hadoop/hive/metastore/ Implementations
of what? Not sure if this method belongs here in the first place.
  metastore/src/java/org/apache/hadoop/hive/metastore/ What
is the plan for removing this interface? Can we remove it on trunk as soon as HCatalog has
migrated to the new interface?


> Merge MetaStoreListener and HiveMetaHook interfaces
> ---------------------------------------------------
>                 Key: HIVE-2720
>                 URL:
>             Project: Hive
>          Issue Type: Sub-task
>          Components: JDBC, Metastore, ODBC, Security
>            Reporter: Enis Soztutar
>            Assignee: Enis Soztutar
>         Attachments: HIVE-2720.D1299.1.patch, HIVE-2720.D1299.2.patch, HIVE-2720.D1299.3.patch
> MetaStoreListener and HiveMetaHook both serve as a notification mechanism for metastore-related
events. The former is used by hcat and the latter is by the hbase-storage handler, and invoked
by the client. 
> I propose to merge these interfaces, and extend the MetaStoreListener, to add most of
the on- and pre- methods at the Thrift interface. This way, extending metastore will be easier,
and validation, storage-driver notification, and enforcement can be delegated to individual
listeners. Besides, more functionality can be plugged-in by Hcat at this level. 

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:!default.jspa
For more information on JIRA, see:


View raw message