> On Sept. 24, 2016, 11:59 p.m., Alexander Kolbasov wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java, > > lines 215-216 > > <https://reviews.apache.org/r/52138/diff/3/?file=1509069#file1509069line215> > > > > fullUpdateComplete variable is only used inside HMSFollower class - I > > would consider moving it inside and making it private. Since it is accessed > > by a single executor thread only there is no need to make it atomic > > (although this wouldn't hurt). > > Hao Hao wrote: > I do not think we shoud change from atomic to non-atomic. > fullUpdateComplete is a shared flag between main thread and HMSFollower. Once > fullUpdateComplete is true, which means full update is complete, HMSFollower > should not retrieve full update again.
Keeping it atomic is fine. But it seems that fullUpdateComplete isn't actually shared - it is only used by HMSFollower thread. That's why I suggested moving the variable inside HMSFollower class. > On Sept. 24, 2016, 11:59 p.m., Alexander Kolbasov wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java, > > lines 215-216 > > <https://reviews.apache.org/r/52138/diff/3/?file=1509069#file1509069line215> > > > > We can't throw exceptions from run() method since it will prevent it > > from ever running again. > > Hao Hao wrote: > During the process of fetching full update, I did not throw any exception > but just returned to the caller. Your code doesn't, but there is existing code that does: } catch (SentryInvalidInputException|SentryInvalidHMSEventException e) { throw new RuntimeException(e); } - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52138/#review150322 ----------------------------------------------------------- On Oct. 4, 2016, 5:42 a.m., Hao Hao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52138/ > ----------------------------------------------------------- > > (Updated Oct. 4, 2016, 5:42 a.m.) > > > Review request for sentry, Alexander Kolbasov, Anne Yu, Li Li, and Sravya > Tirukkovalur. > > > Repository: sentry > > > Description > ------- > > SENTRY-1463: Ensure HMS point-in-time snapshot consistency > > The implemented logic is: > 1. Read current HMS notification ID_initial > 2. Read HMS metadata state > 3. Read current notification ID_new > 4. If ID_initial != ID_new then discard the current state and goto 1. > > Use configurable property: sentry.hms.snapshot.retries.max.count for max > number of retry. > > Change-Id: I7590076b875bd97b2fb340008926ea5995896d72 > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > 894fcc966b511ccf309599fd10960f9a11ae8e96 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java > abc3f58d21bb774427a34399b6e9f51a37ba51db > > Diff: https://reviews.apache.org/r/52138/diff/ > > > Testing > ------- > > > Thanks, > > Hao Hao > >
