----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39228/#review102752 -----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java (line 39) <https://reviews.apache.org/r/39228/#comment160503> Since there vars are not changed, how about add keyword - final? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java (line 41) <https://reviews.apache.org/r/39228/#comment160505> how about rename it to RETRY_COUNT_INTERVAL_SEC_CONF? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java (line 42) <https://reviews.apache.org/r/39228/#comment160506> how about rename it to RETRY_INTERVAL_SEC_DEFAULT? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java (line 46) <https://reviews.apache.org/r/39228/#comment160504> how about rename it to retryIntervalSec? I think it is better to let users know the unit from the name. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java (line 77) <https://reviews.apache.org/r/39228/#comment160509> I think here var 'tries' is redundant. We can rename maxTries to retries, and -1 in while loop. eg. while (retries > 0) { retries--; try{ } catch (Exception e) { // sleep only when retries > 0 } } sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java (line 92) <https://reviews.apache.org/r/39228/#comment160508> how about adding some log here? - Li Li On Oct. 12, 2015, 12:11 p.m., Yibing Shi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39228/ > ----------------------------------------------------------- > > (Updated Oct. 12, 2015, 12:11 p.m.) > > > Review request for sentry and Sravya Tirukkovalur. > > > Repository: sentry > > > Description > ------- > > SENTRY-902: SimpleDBProviderBackend should retry the authrization process > properly > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java > 191e099 > > Diff: https://reviews.apache.org/r/39228/diff/ > > > Testing > ------- > > Test on a local cluster to make sure this retrying logic works. > > > Thanks, > > Yibing Shi > >
