> On Sept. 23, 2016, 4:41 p.m., Alexander Kolbasov wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java, > > lines 239-257 > > <https://reviews.apache.org/r/52138/diff/3/?file=1509069#file1509069line239> > > > > should it be an exception rather than return? Or return false to the > > caller to let it know that update fails. > > Hao Hao wrote: > Not sure if should throw exception or just return. Since the HMSfollower > is scheduled for execute periodically, I think it make sense if cannot get a > full snapshot this time can continue fetching next task?
Agreed. Since it is a periodic thing it doesn't help much to throw an excetion. It is a bit strange that HMSFollower doesn't handle its scheduling internally but requires callers to schedule it instead. But this is outside the scope for this fix. > On Sept. 23, 2016, 4:41 p.m., Alexander Kolbasov wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java, > > lines 239-257 > > <https://reviews.apache.org/r/52138/diff/3/?file=1509069#file1509069line239> > > > > It seems that the whole fullUpdateCOmplete isn't used. > > Hao Hao wrote: > fullUpdateComplete is indicate a full snapshot is fetched and do not need > to get a full snapshot again. I see. It is fine then. > On Sept. 23, 2016, 4:41 p.m., Alexander Kolbasov wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java, > > lines 239-257 > > <https://reviews.apache.org/r/52138/diff/3/?file=1509069#file1509069line239> > > > > Are there any established markers in the log for such events? > > Hao Hao wrote: > We did not have one before. Do you think it make sense to add one for it? At a minimum we should be consistent within Sentry. If there are any common guidelines for other HDFS products we may follow them. If not we should just decide on something and stick to it. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52138/#review150197 ----------------------------------------------------------- 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 > >
