> On Oct. 18, 2018, 2:33 p.m., Karthik Manamcheri wrote: > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java > > Line 244 (original), 341 (patched) > > <https://reviews.apache.org/r/69054/diff/2/?file=2098740#file2098740line347> > > > > You are ignoring the return value? Should you have > > pmf=getUpdatedPmfIfNeeded(..)? > > Vihang Karajgaonkar wrote: > The pmf is updated by the method if needed, so we don't need to use the > return value. Will rename the method to updatePmfIfNeeded to make it more > readable.
And you can have it has void return. > On Oct. 18, 2018, 2:33 p.m., Karthik Manamcheri wrote: > > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestObjectStore.java > > Lines 927 (patched) > > <https://reviews.apache.org/r/69054/diff/2/?file=2098741#file2098741line927> > > > > Is this testing the new read/write lock behavior? As in, would this > > test have failed if you had the previous behavior of a global lock? > > Vihang Karajgaonkar wrote: > This is performance fix. There was nothing broken in the previous > implementation so its hard to come up with a test which break on the previous > code. The purpose of this test to cover the newly added code so that if there > are any obvious bugs they will be seen. If you have more ideas on how can we > add more tests please suggest and I would be happy to add them. Ok sounds fair enough. We could add performance regression tests, like for example, spin up 100 threads all creating ObjectStores in parallel. With your code, they all should pretty much be instantaneous (since they never lock on each other unless config changes). If we accidentally regress, the test could catch it. What do you think? We don't have to do this as part of this ticket. We can do it as a separate perf regression test. - Karthik ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69054/#review209742 ----------------------------------------------------------- On Oct. 16, 2018, 10:47 p.m., Vihang Karajgaonkar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69054/ > ----------------------------------------------------------- > > (Updated Oct. 16, 2018, 10:47 p.m.) > > > Review request for hive, Andrew Sherman, Alan Gates, and Peter Vary. > > > Bugs: HIVE-20740 > https://issues.apache.org/jira/browse/HIVE-20740 > > > Repository: hive-git > > > Description > ------- > > HIVE-20740 : Remove global lock in ObjectStore.setConf method > > > Diffs > ----- > > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java > 66977d79c946f1ac57aacfbe8704d37bfbac3ea3 > > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestObjectStore.java > b74c3048fa2e18adc7f0d7cc813a180d4466fa36 > > > Diff: https://reviews.apache.org/r/69054/diff/2/ > > > Testing > ------- > > > Thanks, > > Vihang Karajgaonkar > >
