> On April 6, 2017, 2:02 p.m., Na Li wrote: > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java > > Line 137 (original), 135 (patched) > > <https://reviews.apache.org/r/58093/diff/1/?file=1681748#file1681748line141> > > > > do you want to print out the value of "retries +1"? Does "retries + 1" > > gives you want you want?
It does - otherwise I'll see that it failed after 0 retries > On April 6, 2017, 2:02 p.m., Na Li wrote: > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java > > Line 322 (original), 319 (patched) > > <https://reviews.apache.org/r/58093/diff/1/?file=1681748#file1681748line327> > > > > do you need to update testing cases in TestFullUpdateInitializer that > > verifies the exception type? Hmm, the tests are passing fine. I guess tests do not check for this. > On April 6, 2017, 2:02 p.m., Na Li wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > > Line 303 (original), 303 (patched) > > <https://reviews.apache.org/r/58093/diff/1/?file=1681749#file1681749line303> > > > > since you changed the possible exception, you need to update the > > commnet at line 303 Good point, this should be updated. > On April 6, 2017, 2:02 p.m., Na Li wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > > Line 310 (original), 310 (patched) > > <https://reviews.apache.org/r/58093/diff/1/?file=1681749#file1681749line310> > > > > It would be easier to debug if you log the beginning of getting full > > snapshot. Sure, we can add the log. > On April 6, 2017, 2:02 p.m., Na Li wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > > Line 312 (original), 312 (patched) > > <https://reviews.apache.org/r/58093/diff/1/?file=1681749#file1681749line312> > > > > My impression is that it takes some time to get the full snapshot. > > Should we measure the time it takes to get the full snapshot? And show the > > duration in the log message? I think we should provide this as a metric. Logs are time stamped - this could be sufficient, but having this as a metric is useful - then we can get it afterwords. I'll file a JIRA on this. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58093/#review171022 ----------------------------------------------------------- On March 31, 2017, 5:07 a.m., Alexander Kolbasov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58093/ > ----------------------------------------------------------- > > (Updated March 31, 2017, 5:07 a.m.) > > > Review request for sentry, Lei Xu, Hao Hao, kalyan kumar kalvagadda, and Na > Li. > > > Bugs: SENTRY-1676 > https://issues.apache.org/jira/browse/SENTRY-1676 > > > Repository: sentry > > > Description > ------- > > SENTRY-1676: FullUpdateInitializer#createInitialUpdate should not throw > RuntimeExceptio > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java > 146cea2b9467ce82b69bbf402933b1aa350bcd46 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > 6c14f5e46aad4223347d8d057188d31efbb68ed8 > > > Diff: https://reviews.apache.org/r/58093/diff/1/ > > > Testing > ------- > > > Thanks, > > Alexander Kolbasov > >
