> On May 11, 2017, 6:04 p.m., Sahil Takiar wrote: > > This is more of a nit: but is get_metastore_uuid the best name, could it > > just be get_metastore_id. do clients really need to know that its a uuid > > vs. just some id
It is actually creating a UUID instead of just another ID (like a integer). I think using uuid instead of id in the name specifies the behavior more acurately and can be used by the clients accordingly. If we use id I think it may be confusing to the clients because they might not know what kind of id is it without actually inspecting it. > On May 11, 2017, 6:04 p.m., Sahil Takiar wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > > Lines 6979 (patched) > > <https://reviews.apache.org/r/59070/diff/2/?file=1714208#file1714208line6979> > > > > Is logging the exception and throwing it necessary? I think it will help in debugging since the client side code does not log the exception. > On May 11, 2017, 6:04 p.m., Sahil Takiar wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java > > Lines 177 (patched) > > <https://reviews.apache.org/r/59070/diff/2/?file=1714211#file1714211line177> > > > > is this the correct logging library? It was an unused import. Removed it. > On May 11, 2017, 6:04 p.m., Sahil Takiar wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java > > Lines 3519 (patched) > > <https://reviews.apache.org/r/59070/diff/2/?file=1714211#file1714211line3519> > > > > what's the point of this method? considering `getMetastoreDbUuid` just > > delegates to it. One of my initial version of the patch has some more meat to it while was changed later on. Refactored it out now. > On May 11, 2017, 6:04 p.m., Sahil Takiar wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java > > Lines 3546 (patched) > > <https://reviews.apache.org/r/59070/diff/2/?file=1714211#file1714211line3546> > > > > will a specific exception be thrown if the guid already exists. then we > > could just catch that exception. The actual exception type was some Datanucleus exception if I remember it right. Looks like none of the other methods catch such exception specifically so kind of used the same logic here as well. > On May 11, 2017, 6:04 p.m., Sahil Takiar wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java > > Lines 3576-3577 (patched) > > <https://reviews.apache.org/r/59070/diff/2/?file=1714211#file1714211line3576> > > > > is this possible? i thought the key name was a unique key Technically it should not be possible as of this patch but I think it would be good to check for this in case of any inadvertent changes/corruption to the schema. - Vihang ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59070/#review174698 ----------------------------------------------------------- On May 11, 2017, 6:56 p.m., Vihang Karajgaonkar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59070/ > ----------------------------------------------------------- > > (Updated May 11, 2017, 6:56 p.m.) > > > Review request for hive, Ashutosh Chauhan, Naveen Gangam, Sergio Pena, and > Sahil Takiar. > > > Bugs: HIVE-16555 > https://issues.apache.org/jira/browse/HIVE-16555 > > > Repository: hive-git > > > Description > ------- > > HIVE-16555 : Add a new thrift API call for get_metastore_db_uuid > > > Diffs > ----- > > > itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java > 88b9faf8394a59de39be55b2dd2315db7a8d5ab4 > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestEmbeddedHiveMetaStore.java > bc00d11e2a1c9fd66b89f1ceca100aaafe43cfed > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java > b95c25ca00751629577e014801c3fb9f1a99bd70 > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestRemoteHiveMetaStore.java > ef02968e22363d537f58b6054266bf9bc87033ae > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestSetUGIOnOnlyClient.java > 29768c1d660aac937c0cd1fa15fb70b571007d14 > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestSetUGIOnOnlyServer.java > 4a46f7537f3ceb16c45010b88786907109fd1090 > metastore/if/hive_metastore.thrift ca6a0076d1fbee4b0d904c1bafcc056ab739e4c4 > metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h > ca71711e09de962d1cd2ee2ee72b3fcbbac228bc > metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp > 9042cdb265373cd25ee9050fb59f6547f4dfc669 > metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp > b4a2a926428d529cc88954552eb561041404877d > metastore/src/gen/thrift/gen-cpp/hive_metastore_types.h > c21ded144484f83cf59b989eada5506ed8e9ba3b > metastore/src/gen/thrift/gen-cpp/hive_metastore_types.cpp > e3725a543ec44ae46f7475156cae270b37b01196 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/MetastoreDBProperty.java > PRE-CREATION > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java > 19151507cae1921a38028582ce00e34cd00585eb > metastore/src/gen/thrift/gen-php/metastore/ThriftHiveMetastore.php > 4fb71839471a1a7b8b8ebd9573212c7d40e9f39d > metastore/src/gen/thrift/gen-php/metastore/Types.php > 74f0028c4124c729385eeee954537e4fdaf992eb > metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote > f2a97997a4aaaf0ed07dc094ee8303717e01284d > metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py > 8ee84af14f87b23956b6bee268c0092e439c17e0 > metastore/src/gen/thrift/gen-py/hive_metastore/ttypes.py > f26cb5b185cf6ae4b79786e6911f1b052822011a > metastore/src/gen/thrift/gen-rb/hive_metastore_types.rb > f1aa9a6a9738d8a2d544c15aaa5c1348a6e2ce6c > metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb > 04e63f3a9b858d79bfa4883c36c2ccce69bf55c4 > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > cbcfc72ac73ccfe48dd9b57eb9722ae092e7094b > > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java > 53f81188c1cda13f20bdf757391024e0c289d9f9 > metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java > 023a2893c3b046999981cccf8e7ff21227601f6b > metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java > a83e12e8f3e3a2f3149e4bbc09524998d0e8928f > metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java > c22a1db046814bf987de0df33b79e718b8fd6dc6 > metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java > 39b1676eb9d3c344a6e66f06f096f3a9fb1931ca > metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java > f6420f5b99fc49df8675afdddd9718871b6c01bb > > metastore/src/model/org/apache/hadoop/hive/metastore/model/MMetastoreDBProperties.java > PRE-CREATION > metastore/src/model/package.jdo 969e19912791b5f5a2b9c5fa4c43800310f5080c > > metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java > 3e3fd20de069503fcedacf60fa1df90279af26b2 > > metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java > 91d8c2af67ae1e49f8d41f16d8eee361b3b2abf9 > > > Diff: https://reviews.apache.org/r/59070/diff/3/ > > > Testing > ------- > > > Thanks, > > Vihang Karajgaonkar > >
