> On June 13, 2017, 11:43 p.m., Alexander Kolbasov wrote: > > The changes for SentryStore are not related - they are just adding extra > > logging - do we really need it as part of this fix? Otherwise LGTM.
These are very important to understand what's going on. I felt the real need of them to debug. It's better to add logging as and when we need identify them. other option is just to create jira to add more logging. - kalyan kumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59887/#review177832 ----------------------------------------------------------- On June 13, 2017, 10:31 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59887/ > ----------------------------------------------------------- > > (Updated June 13, 2017, 10:31 p.m.) > > > Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, > Sergio Pena, Vamsee Yarlagadda, and Vadim Spector. > > > Bugs: SENTRY-1794 > https://issues.apache.org/jira/browse/SENTRY-1794 > > > Repository: sentry > > > Description > ------- > > There were checks like these while processing the alter table and alter > partition notifications which is wrong. This was causing incorrect behavior. > } else if ((oldDbName == newDbName) && > (oldTableName == newTableName) && > (oldLocation == newLocation)) { > Because of such invalid condition check's notification ID is not persisted in > cases where alter table and alter partition notifications are not processed. > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 17856a4 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > 78dc0ac > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/NotificationProcessor.java > de8e2f7 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java > 74a5afb > > > Diff: https://reviews.apache.org/r/59887/diff/2/ > > > Testing > ------- > > Added new tests for make sure that this issue is not seen. > There are upstream tests that are failing. Working on them > > > Thanks, > > kalyan kumar kalvagadda > >
