[
https://issues.apache.org/jira/browse/HIVE-2147?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13038946#comment-13038946
]
jirapos...@reviews.apache.org commented on HIVE-2147:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/738/#review713
-----------------------------------------------------------
trunk/metastore/if/hive_metastore.thrift
<https://reviews.apache.org/r/738/#comment1428>
Having separate calls for sending request and response messages looks
unnecessary. A sendMessage() function with separate request and response
message types should work just as well, and will help to avoid confusion --
otherwise I think people will assume that receiveMessage is a polling call.
This is starting to look like a general purpose messaging/rpc framework. Is
that the intent?
trunk/metastore/if/hive_metastore.thrift
<https://reviews.apache.org/r/738/#comment1429>
Identifying the message type using an integer seems brittle. This won't
work if you have more than one application that is firing events at the
metastore.
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/738/#comment1430>
So the event model is that each event may be handled by at most one event
handler?
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/738/#comment1431>
Please add some DEBUG or TRACE level logging here that indicates which
handler consumed a particular event, or if an event was unserviceable.
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/738/#comment1432>
Semantically this function looks more like "sendRequest" than
"receiveMessage" (and "sendMessage" looks more like "fireEvent").
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/738/#comment1433>
Checkstyle: you need a space between control flow tokens and open parens.
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java
<https://reviews.apache.org/r/738/#comment1434>
Nice to have: javadoc.
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListener.java
<https://reviews.apache.org/r/738/#comment1435>
canProcessSendMessage() looks like a redundant call. Is there any reason
that this can't be be rolled into processSendMessage()?
- Carl
On 2011-05-12 21:03:29, Ashutosh Chauhan wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/738/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-05-12 21:03:29)
bq.
bq.
bq. Review request for hive and Carl Steinbach.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Updated patch to include missing ASF license and generated thrift code.
bq.
bq.
bq. This addresses bug HIVE-2147.
bq. https://issues.apache.org/jira/browse/HIVE-2147
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. trunk/metastore/if/hive_metastore.thrift 1102450
bq. trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h 1102450
bq. trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 1102450
bq.
trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp
1102450
bq.
trunk/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java
1102450
bq.
trunk/metastore/src/gen/thrift/gen-php/hive_metastore/ThriftHiveMetastore.php
1102450
bq.
trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote
1102450
bq.
trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py
1102450
bq. trunk/metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb 1102450
bq.
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
1102450
bq.
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
1102450
bq.
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java
1102450
bq.
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListener.java
1102450
bq.
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/events/MessageEvent.java
PRE-CREATION
bq.
trunk/metastore/src/test/org/apache/hadoop/hive/metastore/DummyListener.java
1102450
bq.
trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java
1102450
bq.
bq. Diff: https://reviews.apache.org/r/738/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Updated TestMetaStoreEventListener to test new api.
bq.
bq.
bq. Thanks,
bq.
bq. Ashutosh
bq.
bq.
> Add api to send / receive message to metastore
> ----------------------------------------------
>
> Key: HIVE-2147
> URL: https://issues.apache.org/jira/browse/HIVE-2147
> Project: Hive
> Issue Type: Improvement
> Components: Metastore
> Reporter: Ashutosh Chauhan
> Assignee: Ashutosh Chauhan
> Fix For: 0.8.0
>
> Attachments: api-without-thrift.patch
>
>
> This is follow-up work on HIVE-2038.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
