----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68805/#review208955 -----------------------------------------------------------
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java Lines 2929 (patched) <https://reviews.apache.org/r/68805/#comment293223> This comment seems confusing to me. Maybe give Kafka offset as a concrete example of point to some wiki where this API is documented. for example, "...for example to know if a transaction has already been committed" which transaction is this talking about? standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java Lines 1095 (patched) <https://reviews.apache.org/r/68805/#comment293218> I think a MetaException would be better (or IllegalState/Argument). SQLException is generally produced by the DB and has sqlstate/sqlcode that various handlers try to examine. standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java Lines 1105 (patched) <https://reviews.apache.org/r/68805/#comment293219> MetaException. Also, it should at least include info to help identify what exactly failed, i.e. txnid, tableid, param/value. W/o it's impossible to correlate this error batch id, etc. I'ld also add a LOG.warn() so that it's visible in the log file. It seems you have a requirement that the parameter exist. Perhaps as part of the error code path, you can do another query to see if does exist - I bet that would be a common error. standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java Lines 2289 (patched) <https://reviews.apache.org/r/68805/#comment293220> why not make (tbleid,key,value) it's own object. Then this object in CommitTxnRequest can be optional but all 3 fields in it can be mandatory. as it is you are checking if they are set here and in TxnHandler.commit... standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTxns.java Lines 101 (patched) <https://reviews.apache.org/r/68805/#comment293221> Nit: what is the advantage of using direct jdbc calls to modify the metastore DBMS. Why not run "cretate table ...", "Alter table..." though Driver and "describe table to see the value" standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTxns.java Lines 135 (patched) <https://reviews.apache.org/r/68805/#comment293222> should probably check that you got the right exception not just "any exception", i.e. check the message. - Eugene Koifman On Sept. 21, 2018, 3:51 p.m., Jaume Marhuenda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68805/ > ----------------------------------------------------------- > > (Updated Sept. 21, 2018, 3:51 p.m.) > > > Review request for hive. > > > Repository: hive-git > > > Description > ------- > > HIVE-20538: Allow to store a key value together with a transaction. > > > Diffs > ----- > > > standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CommitTxnRequest.java > db47f9db8b > > standalone-metastore/metastore-common/src/gen/thrift/gen-php/metastore/Types.php > 22deffe1d3 > > standalone-metastore/metastore-common/src/gen/thrift/gen-py/hive_metastore/ttypes.py > 38fac465d7 > > standalone-metastore/metastore-common/src/gen/thrift/gen-rb/hive_metastore_types.rb > 0192c6da31 > > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java > df6d56b679 > > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java > 54e7eda0da > standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift > 85a5c601e0 > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java > d76049eda1 > > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java > ce590d0f55 > > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTxns.java > db4dd9ec42 > > > Diff: https://reviews.apache.org/r/68805/diff/1/ > > > Testing > ------- > > > Thanks, > > Jaume Marhuenda > >
