----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64545/#review193591 -----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatusMonitor.java Line 133 (original), 132 (patched) <https://reviews.apache.org/r/64545/#comment272160> my 5 cents: I'd personally implement toString() method to print all useful info about this object, something like return String.format("%s{incarnationId=%s, leaderCount=%d, isLeader=%b, isSingleNodeMode=%b}", getClass().getSimpleName(), <fields-to-log>); then, outside the constructor, you can simply log as LOGGER.info("{}: {}", this, your-message); which is shorter. Some people may argue whether we need all these fields logged, but in reality we rarely complain that there is too much information in the logs; usually the opposite. But whichever way you decide, it's already a good improvement. - Vadim Spector On Dec. 12, 2017, 5:10 p.m., Arjun Mishra wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64545/ > ----------------------------------------------------------- > > (Updated Dec. 12, 2017, 5:10 p.m.) > > > Review request for sentry, Brian Towles, kalyan kumar kalvagadda, Na Li, > Sergio Pena, and Vadim Spector. > > > Repository: sentry > > > Description > ------- > > Currently sentry's log messaging when it becomes a 'writer' (the sentry > server that reads from HMS) is not obvious, and is really mostly discernable > at the debug level. Add a log message at the INFO level, such as 'This sentry > server has just become the HMS reader/writer' that is printed once when the > sentry server first becomes the HMS reader/writer. > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatusMonitor.java > 360c5a530 > > > Diff: https://reviews.apache.org/r/64545/diff/3/ > > > Testing > ------- > > mvn -f sentry-provider/sentry-provider-db/pom.xml test > > > Thanks, > > Arjun Mishra > >
