----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29807/#review69751 -----------------------------------------------------------
Hi Dong, This looks great!! Can we rename "RuntimeTimeout" to "Deadline" so it's more clear what this is? Also it appears we are taking the timeout configuration from local config. Do you plan on having the client setting the timeout value they are using somehow? Clients might use varying timeouts. Thanks again! I really like this change. Brock metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java <https://reviews.apache.org/r/29807/#comment114554> an you add VisibleForTest? metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java <https://reviews.apache.org/r/29807/#comment114561> We are mixing old-style TestCase and annotation based testing. We should remove TestCase super class and used @Test, @Before @After metastore/src/test/org/apache/hadoop/hive/metastore/TestRuntimeTimeout.java <https://reviews.apache.org/r/29807/#comment114560> We are mixing TestCase and @Test annotations. I think we should remove the TestCase super class. metastore/src/test/org/apache/hadoop/hive/metastore/TestRuntimeTimeout.java <https://reviews.apache.org/r/29807/#comment114557> let's log this exception metastore/src/test/org/apache/hadoop/hive/metastore/TestRuntimeTimeout.java <https://reviews.apache.org/r/29807/#comment114555> let's log this exception metastore/src/test/org/apache/hadoop/hive/metastore/TestRuntimeTimeout.java <https://reviews.apache.org/r/29807/#comment114556> let's log this exception metastore/src/test/org/apache/hadoop/hive/metastore/TestRuntimeTimeout.java <https://reviews.apache.org/r/29807/#comment114558> let's log this exception and give a better error message metastore/src/test/org/apache/hadoop/hive/metastore/TestRuntimeTimeout.java <https://reviews.apache.org/r/29807/#comment114559> let's log this exception - Brock Noland On Jan. 22, 2015, 8:22 a.m., Dong Chen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29807/ > ----------------------------------------------------------- > > (Updated Jan. 22, 2015, 8:22 a.m.) > > > Review request for hive. > > > Repository: hive-git > > > Description > ------- > > HIVE-9253: MetaStore server should support timeout for long running requests > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5e00575 > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > caad948 > metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java > 564ac8b > metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java > 01ad36a > metastore/src/java/org/apache/hadoop/hive/metastore/RuntimeTimeout.java > PRE-CREATION > > metastore/src/java/org/apache/hadoop/hive/metastore/RuntimeTimeoutException.java > PRE-CREATION > > metastore/src/java/org/apache/hadoop/hive/metastore/SessionPropertiesListener.java > PRE-CREATION > > metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStoreTimeout.java > PRE-CREATION > metastore/src/test/org/apache/hadoop/hive/metastore/TestRuntimeTimeout.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/29807/diff/ > > > Testing > ------- > > UT passed > > > Thanks, > > Dong Chen > >
