----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40615/#review109439 -----------------------------------------------------------
Ship it! lgtm with a few minor comments. sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java (line 45) <https://reviews.apache.org/r/40615/#comment168967> Add TODO to make CallResult immutable (or just make the change if it's not too invasive). sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java (line 68) <https://reviews.apache.org/r/40615/#comment168968> no need to set to null sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java (line 90) <https://reviews.apache.org/r/40615/#comment168966> Should "exception" be reset to null after each loop or do we want to pass in an exception from a previous retry to the successful call? sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java (line 92) <https://reviews.apache.org/r/40615/#comment168970> change to LOGGER.debug("<MESSAGE>", ex); Also consider adding a more useful message such as: "Failed to execute task on attempt N/Y. Sleeping for <> ms. Error: ", exception" - Lenni Kuff On Dec. 9, 2015, 1:56 a.m., Hao Hao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40615/ > ----------------------------------------------------------- > > (Updated Dec. 9, 2015, 1:56 a.m.) > > > Review request for sentry, Anne Yu, Lenni Kuff, and Sravya Tirukkovalur. > > > Repository: sentry > > > Description > ------- > > Change-Id: I3f9e2e1f17e8ff6d336ca748d2be763cc767bd72 > > Retries the failed task with configurable times. Each retry waits for a > configurable times. > Client can configure whether to fail the startup (atomic) or continue with > partial paths with log errors available. > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java > 8f62496155b2445ab677f77be28a7ddafba0a029 > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java > eb85d45d9401cfa25b01bf70b860c9d3d940dd52 > > sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestMetastoreCacheInitializer.java > f1e729ff9fd94cfbcd9d0ba89d850b665e681904 > > Diff: https://reviews.apache.org/r/40615/diff/ > > > Testing > ------- > > Tested on TestMetastoreCacheInitializer. > > > Thanks, > > Hao Hao > >
