> On May 15, 2017, 3:14 p.m., Alexander Kolbasov wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > > Lines 328 (patched) > > <https://reviews.apache.org/r/59274/diff/1/?file=1718347#file1718347line329> > > > > note that getting eventId and getting events is done via to separate > > calls. New events may appear between these two calls, so it is very likely > > that the condition > > > > response.getEvents().size() == expectedNotificationsSize) > > > > is false. > > Sergio Pena wrote: > True. What about just checking if expected notifications is LESS instead? > If we get less events, then something is wrong. If we get more events, then > we're fine. > > Alexander Kolbasov wrote: > What are you trying to achieve here? Asl long as we know that there are > some new events to process, is there a reason not to process them? The case > where we get LESS events that we'd hope we would get is, indeed, weird, but > it doesn't seem to warrant a full snapshot, or you think otherwise?
I believe Sergio considers (expected notification size > retrived events size) equal to (current eventId < min(Notification ID) at meta store) (which inditicates a gap and some notifications are removed from meta store and not received by Sentry). This equivalence seems OK. One problem is that it assumes the meta store returns all notifications from [current eventID, max(Notification ID)]. Is it possible meta store sends only part of the notification because they are too big? - Na ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59274/#review174953 ----------------------------------------------------------- On May 15, 2017, 2:58 p.m., Sergio Pena wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59274/ > ----------------------------------------------------------- > > (Updated May 15, 2017, 2:58 p.m.) > > > Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and > Na Li. > > > Bugs: SENTRY-1760 > https://issues.apache.org/jira/browse/SENTRY-1760 > > > Repository: sentry > > > Description > ------- > > The patch will set the 'needHiveSnapshot' to TRUE whenever the following > cases are found: > > - List of notifications received are different than expected. > This may happen when Sentry has been down or HDFS sync was disabled for a > while (more than 24h), > and the HMS cleared old notifications (older than 24h) not processed by > Sentry causing a gap when retrieving notifications. > > - Latest Sentry notification ID processed is bigger than current HMS > notification ID. > This may happen when the HMS DB data was reset or restore from an old > snapshot causing sync issues with Sentry. > > When needHiveSnapshot is set to TRUE, then the HMSFollower will CLEAR any > hive snapshot stored on the Sentry store, and recreate a > new hive snapshot from scratch to keep Sentry in sync. > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > ef6786537e9c5f7730bc86d44e8b4a168c20677e > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > 5e6b906587f6422d9bf1466ab83815722bd51fb0 > > > Diff: https://reviews.apache.org/r/59274/diff/1/ > > > Testing > ------- > > HadoopQA is GREEN. > > > Thanks, > > Sergio Pena > >
