----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62221/#review185234 -----------------------------------------------------------
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java Lines 69 (patched) <https://reviews.apache.org/r/62221/#comment261527> Does it hurt if we explicitly print the entire classname in the log than having to maintain this extra logic just for log? Now the log message would look like: Before ```bash 2017-09-12 13:29:14,459 INFO org.apache.sentry.hdfs.DBUpdateForwarder: Newer delta updates are found up to sequence number: 36342 ``` After ```bash 2017-09-12 13:29:14,459 INFO org.apache.sentry.hdfs.DBUpdateForwarder: (org.apache.sentry.hdfs.PathImageRetriever) Newer delta updates are found up to sequence number: 36342 ``` sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java Lines 112 (patched) <https://reviews.apache.org/r/62221/#comment261528> Should we reword and say "A newer full update with image number {} was found, loaded and being sent to HDFS?" sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java Lines 91 (patched) <https://reviews.apache.org/r/62221/#comment261529> We should also reset this to true whenever the server becomes the leader making HMSFollower active to process snapshots/notifications. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java Lines 180-183 (patched) <https://reviews.apache.org/r/62221/#comment261530> This part is evident from the logs that Sentry connects to HMS (SentryHMSClient prints it). What's valuable is to print HMSFollower is all good at the end of first iteration. I expect this to be printed after line 208 (processNotifications(notifications);) which marks the end of HMSFollower iteration. I would expect a log message to tell me everything went well so far without any issues. Makes sense? - Vamsee Yarlagadda On Sept. 12, 2017, 2:36 p.m., Arjun Mishra wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62221/ > ----------------------------------------------------------- > > (Updated Sept. 12, 2017, 2:36 p.m.) > > > Review request for sentry, Alexander Kolbasov, Brian Towles, Na Li, Sergio > Pena, and Vamsee Yarlagadda. > > > Repository: sentry > > > Description > ------- > > Sentry Supportability improvement. > > 1. HMSFollower > * Print confirmation message (at INFO level) once full snapshot is persisted > in the DB. > * Print the message that HMSFollower is completely ready (after the initial > pass of HMSFollower is done) > > 2. DBUpdateForwarder > * Every log message should explicitly mention which type of events is it > referring to (PERM or PATH) otherwise there is no way for us to differentiate > between calls. > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java > 8a34d5623 > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java > 1318082d3 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > 31fd4597d > > > Diff: https://reviews.apache.org/r/62221/diff/4/ > > > Testing > ------- > > > Thanks, > > Arjun Mishra > >
