----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62107/#review184996 -----------------------------------------------------------
Fix it, then Ship it! The code looks good. I just have a couple of comments below. But I think we're good to go with this patch. sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java Line 105 (original), 107 (patched) <https://reviews.apache.org/r/62107/#comment261228> Why did you change the constant here? I'm trying to remember why we left this condition here. The old constan is -1 but the new you changed is 0. I think we were trying to keep some compatibility with an old issue we saw on the HDFS requests, but I don't remember. sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java Lines 123 (patched) <https://reviews.apache.org/r/62107/#comment261229> Previous log messages have INFO level. I think we should set this as INFO as well so that users know that a snapshot is requested but the client will be delayed. - Sergio Pena On Sept. 8, 2017, 4:29 p.m., Brian Towles wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62107/ > ----------------------------------------------------------- > > (Updated Sept. 8, 2017, 4:29 p.m.) > > > Review request for sentry, Alexander Kolbasov, Arjun Mishra, Na Li, Sergio > Pena, and Vadim Spector. > > > Bugs: SENTRY-1918 > https://issues.apache.org/jira/browse/SENTRY-1918 > > > Repository: sentry > > > Description > ------- > > Check the state if the SentryService to see if its doing a full update and > delay the NN requests for a full update while thats going on. > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java > 0ff717d68f146459862806d6d813f0131949777f > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java > bb85c1381b6cd5126148596bb1637f9b3a9de5fd > > sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestDBUpdateForwarder.java > 830d00eb467b47ef2abcd9f55ec0133a9d395518 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > 4d83ad5262e6c1a5bae3dc290a00bf9cc1a91faa > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollowerState.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > d44abffc199cd46d3ab7d2230dfdb2075736a8f0 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceState.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryState.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryStateBank.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryStateBankTestHelper.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestSentryStateBank.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/62107/diff/7/ > > > Testing > ------- > > Unit Tests > > > Thanks, > > Brian Towles > >
