----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40615/#review108884 -----------------------------------------------------------
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java (line 56) <https://reviews.apache.org/r/40615/#comment168344> I would assume that a RetryStrategy would just story the information needed to perform a retry, but this class seems to be doing a bit more - sleeping, tracking the retry count, storing exceptions, etc. Two options: 1) You can simplify this class (make it immutable) and put more of the logic in the caller. 2) Rename this to something like RetryableAction and pass in a callable object. Then the caller would do something like: Callable callable = new Callable(); RetryableAction action = new RetryableAction(retryCount, retrySleepMs, c); // Executes the action with the specified retry logic. action.exec(); I think either approach is fine. 2) might be a bit of overkill, but could be reused in more places. - Lenni Kuff On Nov. 23, 2015, 10:17 p.m., Hao Hao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40615/ > ----------------------------------------------------------- > > (Updated Nov. 23, 2015, 10:17 p.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 time. > 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 > 19b0b49a14649f8da7ab5e00e38216e3ba9c15e2 > > 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 > >
