----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58284/#review171443 -----------------------------------------------------------
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java Lines 220 (patched) <https://reviews.apache.org/r/58284/#comment244379> can we log the number of retires in this warning message? sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java Line 128 (original), 221 (patched) <https://reviews.apache.org/r/58284/#comment244377> should retries = i + 1? sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java Line 129 (original), 222 (patched) <https://reviews.apache.org/r/58284/#comment244378> should we use "break" to get out of the for() loop instead of setting "i = retryStrategyMaxRetries;"? In current implementation, the line 226 "retries = i;" will be called when InterruptedException is called, then "retries" will be set as "retryStrategyMaxRetries". That is not correct sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestFullUpdateInitializer.java Line 125 (original), 214 (patched) <https://reviews.apache.org/r/58284/#comment244380> can we have the test case that the first time, client returns exception, but the second time, it returns correct result. This is to test the retry behavior. - Na Li On April 10, 2017, 5:42 a.m., Alexander Kolbasov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58284/ > ----------------------------------------------------------- > > (Updated April 10, 2017, 5:42 a.m.) > > > Review request for sentry, Hao Hao, kalyan kumar kalvagadda, Na Li, Sergio > Pena, Vamsee Yarlagadda, and Vadim Spector. > > > Bugs: SENTRY-1687 > https://issues.apache.org/jira/browse/SENTRY-1687 > > > Repository: sentry > > > Description > ------- > > SENTRY-1687 FullUpdateInitializer can be more efficient > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java > 90aaaef0e15306627d7108f12a74a29848055c0b > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java > 14e967aa1065f16e8d4c3f61db2f9055959fa9e6 > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java > 23552c2512902a8500bfacb1c745ca4b10498cc8 > > sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestFullUpdateInitializer.java > f338ce8abddcde128536a0efef77983990325aa6 > > sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java > b5cbea9d295385bb38b912c13903edace04d7589 > > sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestUpdateableAuthzPaths.java > e643e01a45de77ef7f465d6921c5ae9e7ce925a2 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > 16676fb13b0d5015aefe892a6f7e46812ea75124 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java > 24ab1a8b392f23bc75759733bef7cecd4bc2ac34 > > > Diff: https://reviews.apache.org/r/58284/diff/1/ > > > Testing > ------- > > Updated the unit test to test for bigger HMS snapshots > > > Thanks, > > Alexander Kolbasov > >
