----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62229/#review185121 -----------------------------------------------------------
Fix it, then Ship it! The patch looks good overall. I just left a couple of minor comments. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java Lines 1170 (patched) <https://reviews.apache.org/r/62229/#comment261349> The HMS client may display many log errors if we get timeouts too often, but these are not supposed to be errors as HMS will continue working as expected. Should use a WARNING message instead of an ERROR? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/CounterWait.java Lines 39 (patched) <https://reviews.apache.org/r/62229/#comment261348> Isn't {@link TimeoutException} instead? - Sergio Pena On Sept. 11, 2017, 9:35 p.m., Alexander Kolbasov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62229/ > ----------------------------------------------------------- > > (Updated Sept. 11, 2017, 9:35 p.m.) > > > Review request for sentry, Arjun Mishra, Brian Towles, Na Li, Sergio Pena, > and Vamsee Yarlagadda. > > > Bugs: SENTRY-1940 > https://issues.apache.org/jira/browse/SENTRY-1940 > > > Repository: sentry > > > Description > ------- > > SENTRY-1940 Sentry should time out threads waiting for notifications > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > a70a552a837263935ad84d7bd9a2aa507a2eb21e > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java > cfd0e30c911f15a20b7d855d1bfa9a519142062a > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/CounterWait.java > 2c9e87a386a5e4f642a55bcd4636ae4c0752aa0d > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java > 48aec1e80285b5a0184e771ccba35c762f813c59 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestCounterWait.java > e4846d9aea5ef9882fc655de9ec38c5795bde896 > > > Diff: https://reviews.apache.org/r/62229/diff/1/ > > > Testing > ------- > > > Thanks, > > Alexander Kolbasov > >
