> On May 6, 2014, 12:23 a.m., Robert Kanter wrote: > >
Thanks, for reviewing. We also added option to rest sequence once it reach limit. > On May 6, 2014, 12:23 a.m., Robert Kanter wrote: > > core/src/main/java/org/apache/oozie/service/DistributedUUIDService.java, > > line 44 > > <https://reviews.apache.org/r/21046/diff/2/?file=574024#file574024line44> > > > > I think we should use a different RetryPolicy here. RetryOneTime will > > only retry once (with a 1 sec wait) if it fails to increment the ID; in > > other words, Oozie will fail to submit the job if it can't increment the ID > > twice in a row (I suppose that could happen with lots of busy Oozies?) > > > > I think we should use the ExponentialBackoffRetry like I used in > > ZKUtils: > > https://github.com/apache/oozie/blob/master/core/src/main/java/org/apache/oozie/util/ZKUtils.java#L156 > > ZKUtils is set to retry 3 times with an exponentially increasing wait, > > which is more likely to succeed (wireless router do something similar when > > they get collisions) I will add a function to ZKUtils to return retry policy, so all can use same policy ( we might have a case where we need different retry policy for specific scenario, we can modify ZKUtils when we have such requirement). > On May 6, 2014, 12:23 a.m., Robert Kanter wrote: > > core/src/main/java/org/apache/oozie/service/DistributedUUIDService.java, > > lines 66-67 > > <https://reviews.apache.org/r/21046/diff/2/?file=574024#file574024line66> > > > > This could be a race condition with other Oozie servers. > > For example: > > - ID starts at 0 > > - Server A calls get() and receives 0 > > - Server B calls get() and receives 0 > > - Server A calls increment() > > - ID becomes 1 > > - Server B calls increment() > > - ID becomes 2 > > While the end value for the ID is correct, both Servers will get 0 for > > their ID, which will be bad. > > > > Instead, I think you just need to do this: > > value = atomicIdGenerator.increment(); > > (You're already checking value.succeeded() and returning > > value.preValue()) Intent was increment and get the value, but somehow missed that. Thanks for pointing out. - Purshotam ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21046/#review42223 ----------------------------------------------------------- On May 3, 2014, 12:21 a.m., Purshotam Shah wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21046/ > ----------------------------------------------------------- > > (Updated May 3, 2014, 12:21 a.m.) > > > Review request for oozie. > > > Bugs: OOZIE-1715 > https://issues.apache.org/jira/browse/OOZIE-1715 > > > Repository: oozie-git > > > Description > ------- > > OOZIE-1715 Distributed ID sequence for HA > > > Diffs > ----- > > core/src/main/java/org/apache/oozie/service/DistributedUUIDService.java > e69de29 > core/src/main/java/org/apache/oozie/service/UUIDService.java 836815d > core/src/test/java/org/apache/oozie/service/TestDistributedUUIDService.java > e69de29 > > Diff: https://reviews.apache.org/r/21046/diff/ > > > Testing > ------- > > UTC > > > Thanks, > > Purshotam Shah > >
