----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63881/#review192140 -----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 2840 (patched) <https://reviews.apache.org/r/63881/#comment270164> Note that you are loggint this before transaction committed, (or even before you write actual snapshot) so it may be possible that you'll see the log that snapshot is created but it would not be actually created due to some issue with transaction commit. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Line 2912 (original), 2913 (patched) <https://reviews.apache.org/r/63881/#comment270165> It would be also useful to log currentSnapshotID. Also, this is probably warning, not error. Do you want to log the full authzObject here or jost some relevant info from it? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java Lines 424 (patched) <https://reviews.apache.org/r/63881/#comment270166> Message can be quite big - do we really want to log it? - Alexander Kolbasov On Nov. 29, 2017, 5:28 p.m., Arjun Mishra wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63881/ > ----------------------------------------------------------- > > (Updated Nov. 29, 2017, 5:28 p.m.) > > > Review request for sentry, kalyan kumar kalvagadda, Na Li, Sergio Pena, and > Vadim Spector. > > > Repository: sentry > > > Description > ------- > > Currently we only seem to log when all conditions that lead to getting a > snapshot are validated, and eventually a full snapshot is received. However, > if for some reason a particular condition was invalid we don't log as to why. > This leaves no room for knowing definitely what might have been the reason as > to why a snapshot was not received from HMS. > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > f32a745ed > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > c1471d118 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java > eccb40fb6 > > > Diff: https://reviews.apache.org/r/63881/diff/4/ > > > Testing > ------- > > mvn -f sentry-provider/sentry-provider-db/pom.xml test > > > Thanks, > > Arjun Mishra > >