You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Purshotam Shah <pu...@yahoo-inc.com> on 2014/05/03 02:21:34 UTC
Review Request 21046: OOZIE-1715 Distributed ID sequence for HA
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21046/
-----------------------------------------------------------
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
Re: Review Request 21046: OOZIE-1715 Distributed ID sequence for HA
Posted by Purshotam Shah <pu...@yahoo-inc.com>.
> 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
>
>
Re: Review Request 21046: OOZIE-1715 Distributed ID sequence for HA
Posted by Robert Kanter <rk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21046/#review42223
-----------------------------------------------------------
core/src/main/java/org/apache/oozie/service/DistributedUUIDService.java
<https://reviews.apache.org/r/21046/#comment76012>
All of the other HA Services are named ZKSomethingService; can we name this either ZKUUIDService or ZKDistributedUUIDService so it matches and looks like it belongs with them?
core/src/main/java/org/apache/oozie/service/DistributedUUIDService.java
<https://reviews.apache.org/r/21046/#comment76014>
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)
core/src/main/java/org/apache/oozie/service/DistributedUUIDService.java
<https://reviews.apache.org/r/21046/#comment76019>
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())
core/src/test/java/org/apache/oozie/service/TestDistributedUUIDService.java
<https://reviews.apache.org/r/21046/#comment76021>
This should be DistributedUUIDService (or whatever we're calling it), not ZKJobsConcurrencyService
core/src/test/java/org/apache/oozie/service/TestDistributedUUIDService.java
<https://reviews.apache.org/r/21046/#comment76024>
Can you add a test that creates two DistributedUUIDServices (or whatever we're calling it) and tests for race conditions between the two? e.g. if you call them at the same time, they don't get the same ID
- Robert Kanter
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
>
>
Re: Review Request 21046: OOZIE-1715 Distributed ID sequence for HA
Posted by Purshotam Shah <pu...@yahoo-inc.com>.
> On May 19, 2014, 6:57 p.m., Robert Kanter wrote:
> > core/src/main/java/org/apache/oozie/service/ZKUUIDService.java, line 60
> > <https://reviews.apache.org/r/21046/diff/4/?file=582685#file582685line60>
> >
> > What happens if the user sets this too high (i.e. more than the number of digits we use for the sequence part of the ID)?
> >
> > What happens if the user sets this too low (i.e. a negative number, or say, 5)?
> >
> > Perhaps we should have some hardcoded limits on the range of this?
>What happens if the user sets this too low (i.e. a negative number, or say, 5)?
I don't think any body will set -ve value for sequence, if we go that way there will be so many scenario to consider.
This is how ID looks like.
Id sequence
-
yyMMddHHmmssSSS - server start time.
-
system id - systemId = conf.get(CONF_SYSTEM_ID, ("oozie-" + System.getProperty("user.name")));
-
w/c/b.
total should not exceed 40 char.
So there may not be hard limit, if user configure systemID with less char then he can have higher sequence.
Its more for a user to calculate.
For HA, i will take out "SSS" from start time, so that we can higher sequence.
- Purshotam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21046/#review43396
-----------------------------------------------------------
On May 15, 2014, 6:43 p.m., Purshotam Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21046/
> -----------------------------------------------------------
>
> (Updated May 15, 2014, 6:43 p.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/main/java/org/apache/oozie/service/ZKUUIDService.java e69de29
> core/src/main/java/org/apache/oozie/util/ZKUtils.java 56055b8
> core/src/test/java/org/apache/oozie/service/TestDistributedUUIDService.java e69de29
> core/src/test/java/org/apache/oozie/service/TestZKUUIDService.java e69de29
>
> Diff: https://reviews.apache.org/r/21046/diff/
>
>
> Testing
> -------
>
> UTC
>
>
> Thanks,
>
> Purshotam Shah
>
>
Re: Review Request 21046: OOZIE-1715 Distributed ID sequence for HA
Posted by Robert Kanter <rk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21046/#review43396
-----------------------------------------------------------
Can you add the new config properties to oozie-default and/or oozie-site?
Can you also update the HA install documentation?
core/src/main/java/org/apache/oozie/service/ZKUUIDService.java
<https://reviews.apache.org/r/21046/#comment77487>
What happens if the user sets this too high (i.e. more than the number of digits we use for the sequence part of the ID)?
What happens if the user sets this too low (i.e. a negative number, or say, 5)?
Perhaps we should have some hardcoded limits on the range of this?
core/src/main/java/org/apache/oozie/service/ZKUUIDService.java
<https://reviews.apache.org/r/21046/#comment77486>
alread(y) what?
- Robert Kanter
On May 15, 2014, 6:43 p.m., Purshotam Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21046/
> -----------------------------------------------------------
>
> (Updated May 15, 2014, 6:43 p.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/main/java/org/apache/oozie/service/ZKUUIDService.java e69de29
> core/src/main/java/org/apache/oozie/util/ZKUtils.java 56055b8
> core/src/test/java/org/apache/oozie/service/TestDistributedUUIDService.java e69de29
> core/src/test/java/org/apache/oozie/service/TestZKUUIDService.java e69de29
>
> Diff: https://reviews.apache.org/r/21046/diff/
>
>
> Testing
> -------
>
> UTC
>
>
> Thanks,
>
> Purshotam Shah
>
>
Re: Review Request 21046: OOZIE-1715 Distributed ID sequence for HA
Posted by Robert Kanter <rk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21046/#review44575
-----------------------------------------------------------
Ship it!
Ship It!
- Robert Kanter
On May 20, 2014, 12:47 a.m., Purshotam Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21046/
> -----------------------------------------------------------
>
> (Updated May 20, 2014, 12:47 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/main/java/org/apache/oozie/service/ZKUUIDService.java e69de29
> core/src/main/java/org/apache/oozie/util/ZKUtils.java 56055b8
> core/src/main/resources/oozie-default.xml 9788daf
> core/src/test/java/org/apache/oozie/service/TestDistributedUUIDService.java e69de29
> core/src/test/java/org/apache/oozie/service/TestZKUUIDService.java e69de29
> docs/src/site/twiki/AG_Install.twiki e343d7e
>
> Diff: https://reviews.apache.org/r/21046/diff/
>
>
> Testing
> -------
>
> UTC
>
>
> Thanks,
>
> Purshotam Shah
>
>
Re: Review Request 21046: OOZIE-1715 Distributed ID sequence for HA
Posted by Purshotam Shah <pu...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21046/
-----------------------------------------------------------
(Updated May 20, 2014, 12:47 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 (updated)
-----
core/src/main/java/org/apache/oozie/service/DistributedUUIDService.java e69de29
core/src/main/java/org/apache/oozie/service/UUIDService.java 836815d
core/src/main/java/org/apache/oozie/service/ZKUUIDService.java e69de29
core/src/main/java/org/apache/oozie/util/ZKUtils.java 56055b8
core/src/main/resources/oozie-default.xml 9788daf
core/src/test/java/org/apache/oozie/service/TestDistributedUUIDService.java e69de29
core/src/test/java/org/apache/oozie/service/TestZKUUIDService.java e69de29
docs/src/site/twiki/AG_Install.twiki e343d7e
Diff: https://reviews.apache.org/r/21046/diff/
Testing
-------
UTC
Thanks,
Purshotam Shah
Re: Review Request 21046: OOZIE-1715 Distributed ID sequence for HA
Posted by Purshotam Shah <pu...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21046/
-----------------------------------------------------------
(Updated May 15, 2014, 6:43 p.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 (updated)
-----
core/src/main/java/org/apache/oozie/service/DistributedUUIDService.java e69de29
core/src/main/java/org/apache/oozie/service/UUIDService.java 836815d
core/src/main/java/org/apache/oozie/service/ZKUUIDService.java e69de29
core/src/main/java/org/apache/oozie/util/ZKUtils.java 56055b8
core/src/test/java/org/apache/oozie/service/TestDistributedUUIDService.java e69de29
core/src/test/java/org/apache/oozie/service/TestZKUUIDService.java e69de29
Diff: https://reviews.apache.org/r/21046/diff/
Testing
-------
UTC
Thanks,
Purshotam Shah
Re: Review Request 21046: OOZIE-1715 Distributed ID sequence for HA
Posted by Purshotam Shah <pu...@yahoo-inc.com>.
> On May 13, 2014, 9:55 p.m., Robert Kanter wrote:
> > ID's usually look like this: 0000000-140505141334644-oozie-oozi-W
> > where the second value is a timestamp and the first value is the increasing part.
> >
> > With the ZKUUIDService, will all Oozie servers use the same timestamp part? If not, then having the first part shared among the servers doesn't get you too much; so I think we need to have a way of making the timestamp shared as well.
> >
> > Also, when you reset the sequence, isn't it possible to still cause collisions if someone has a really old job still sitting around? I agree that it's probably not likely given the number of jobs they'd have to run without cleaning up, but still. If you also reset the timestamp to "now" when reseting the atomic long, that should take care of this.
>With the ZKUUIDService, will all Oozie servers use the same timestamp part? If not, then having the first part shared among the servers doesn't get you too much; so >I think we need to have a way of making the timestamp shared as well.
In HA Timestamp may tell, where job was submitted.
>Also, when you reset the sequence, isn't it possible to still cause collisions if someone has a really old job still sitting around? I agree that it's probably not >likely given the number of jobs they'd have to run without cleaning up, but still. If you also reset the timestamp to "now" when reseting the atomic long, that >should take care of this.
Once we restart, timestamp will change. If one knows that there are huge jobs submission and less purging, they can increase max count.
Reseting timestamp to "now" might take out the purpose of appending timestamp to jobid.
I believe taking out timestamp for HA is debatable. I will create a JIRA and if all agrees we can take it out.
> On May 13, 2014, 9:55 p.m., Robert Kanter wrote:
> > core/src/main/java/org/apache/oozie/service/ZKUUIDService.java, line 55
> > <https://reviews.apache.org/r/21046/diff/3/?file=580589#file580589line55>
> >
> > I'm not sure we should make the path configurable; I'm concerned that will just lead to a user accidentally setting them different in different servers and then running into problems because Oozie servers are using different paths. Or what happens if they try to use a patch Oozie is already using for something else?
> >
> > Also, please verify that when you run Oozie with ZKUUIDService, the path gets created under the namespace set for the server (default "oozie") and not simply at the root. e.g. /oozie/job_id_sequence
> >
> > Also, can you add a class javadoc comment to ZKUUIDService mentioning the path it's using (see ZKUtils and ZKLocksService for an example of what I mean)
>I'm not sure we should make the path configurable; I'm concerned that will just lead to a user accidentally setting them different in different servers and then >running into problems because Oozie servers are using different paths. Or what happens if they try to use a patch Oozie is already using for something else?
Even Namespace is configurable, it should be ok. Most of the people will verify their setting before moving to production.
I have verified and our QE will also verify that. Path are created under namespace.
> On May 13, 2014, 9:55 p.m., Robert Kanter wrote:
> > core/src/main/java/org/apache/oozie/service/ZKUUIDService.java, line 90
> > <https://reviews.apache.org/r/21046/diff/3/?file=580589#file580589line90>
> >
> > I think you mean "increment" not "reset" here?
I mean reset, we try to reset three time, before throwing exception.
- Purshotam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21046/#review42891
-----------------------------------------------------------
On May 13, 2014, 5:32 p.m., Purshotam Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21046/
> -----------------------------------------------------------
>
> (Updated May 13, 2014, 5:32 p.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/main/java/org/apache/oozie/service/ZKUUIDService.java e69de29
> core/src/main/java/org/apache/oozie/util/ZKUtils.java 56055b8
> core/src/test/java/org/apache/oozie/service/TestDistributedUUIDService.java e69de29
> core/src/test/java/org/apache/oozie/service/TestZKUUIDService.java e69de29
>
> Diff: https://reviews.apache.org/r/21046/diff/
>
>
> Testing
> -------
>
> UTC
>
>
> Thanks,
>
> Purshotam Shah
>
>
Re: Review Request 21046: OOZIE-1715 Distributed ID sequence for HA
Posted by Robert Kanter <rk...@cloudera.com>.
> On May 13, 2014, 9:55 p.m., Robert Kanter wrote:
> > core/src/main/java/org/apache/oozie/service/ZKUUIDService.java, line 55
> > <https://reviews.apache.org/r/21046/diff/3/?file=580589#file580589line55>
> >
> > I'm not sure we should make the path configurable; I'm concerned that will just lead to a user accidentally setting them different in different servers and then running into problems because Oozie servers are using different paths. Or what happens if they try to use a patch Oozie is already using for something else?
> >
> > Also, please verify that when you run Oozie with ZKUUIDService, the path gets created under the namespace set for the server (default "oozie") and not simply at the root. e.g. /oozie/job_id_sequence
> >
> > Also, can you add a class javadoc comment to ZKUUIDService mentioning the path it's using (see ZKUtils and ZKLocksService for an example of what I mean)
>
> Purshotam Shah wrote:
> >I'm not sure we should make the path configurable; I'm concerned that will just lead to a user accidentally setting them different in different servers and then >running into problems because Oozie servers are using different paths. Or what happens if they try to use a patch Oozie is already using for something else?
>
> Even Namespace is configurable, it should be ok. Most of the people will verify their setting before moving to production.
>
>
> I have verified and our QE will also verify that. Path are created under namespace.
>
>
>
Thanks for verifying it's under the namespace.
I'm still not sure it's a good idea to make this path configurable; it's just another thing that the user can mix up between Oozie servers and run into problems. None of the other things are configurable (e.g. ZK Locks go under /locks, service discovery is under /services/servers, etc). Why do you want this to be configurable?
- Robert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21046/#review42891
-----------------------------------------------------------
On May 15, 2014, 6:43 p.m., Purshotam Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21046/
> -----------------------------------------------------------
>
> (Updated May 15, 2014, 6:43 p.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/main/java/org/apache/oozie/service/ZKUUIDService.java e69de29
> core/src/main/java/org/apache/oozie/util/ZKUtils.java 56055b8
> core/src/test/java/org/apache/oozie/service/TestDistributedUUIDService.java e69de29
> core/src/test/java/org/apache/oozie/service/TestZKUUIDService.java e69de29
>
> Diff: https://reviews.apache.org/r/21046/diff/
>
>
> Testing
> -------
>
> UTC
>
>
> Thanks,
>
> Purshotam Shah
>
>
Re: Review Request 21046: OOZIE-1715 Distributed ID sequence for HA
Posted by Robert Kanter <rk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21046/#review42891
-----------------------------------------------------------
ID's usually look like this: 0000000-140505141334644-oozie-oozi-W
where the second value is a timestamp and the first value is the increasing part.
With the ZKUUIDService, will all Oozie servers use the same timestamp part? If not, then having the first part shared among the servers doesn't get you too much; so I think we need to have a way of making the timestamp shared as well.
Also, when you reset the sequence, isn't it possible to still cause collisions if someone has a really old job still sitting around? I agree that it's probably not likely given the number of jobs they'd have to run without cleaning up, but still. If you also reset the timestamp to "now" when reseting the atomic long, that should take care of this.
core/src/main/java/org/apache/oozie/service/UUIDService.java
<https://reviews.apache.org/r/21046/#comment76819>
Why append this twice?
core/src/main/java/org/apache/oozie/service/ZKUUIDService.java
<https://reviews.apache.org/r/21046/#comment76816>
I'm not sure we should make the path configurable; I'm concerned that will just lead to a user accidentally setting them different in different servers and then running into problems because Oozie servers are using different paths. Or what happens if they try to use a patch Oozie is already using for something else?
Also, please verify that when you run Oozie with ZKUUIDService, the path gets created under the namespace set for the server (default "oozie") and not simply at the root. e.g. /oozie/job_id_sequence
Also, can you add a class javadoc comment to ZKUUIDService mentioning the path it's using (see ZKUtils and ZKLocksService for an example of what I mean)
core/src/main/java/org/apache/oozie/service/ZKUUIDService.java
<https://reviews.apache.org/r/21046/#comment76808>
I think you mean "increment" not "reset" here?
- Robert Kanter
On May 13, 2014, 5:32 p.m., Purshotam Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21046/
> -----------------------------------------------------------
>
> (Updated May 13, 2014, 5:32 p.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/main/java/org/apache/oozie/service/ZKUUIDService.java e69de29
> core/src/main/java/org/apache/oozie/util/ZKUtils.java 56055b8
> core/src/test/java/org/apache/oozie/service/TestDistributedUUIDService.java e69de29
> core/src/test/java/org/apache/oozie/service/TestZKUUIDService.java e69de29
>
> Diff: https://reviews.apache.org/r/21046/diff/
>
>
> Testing
> -------
>
> UTC
>
>
> Thanks,
>
> Purshotam Shah
>
>
Re: Review Request 21046: OOZIE-1715 Distributed ID sequence for HA
Posted by Purshotam Shah <pu...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21046/
-----------------------------------------------------------
(Updated May 13, 2014, 5:32 p.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 (updated)
-----
core/src/main/java/org/apache/oozie/service/DistributedUUIDService.java e69de29
core/src/main/java/org/apache/oozie/service/UUIDService.java 836815d
core/src/main/java/org/apache/oozie/service/ZKUUIDService.java e69de29
core/src/main/java/org/apache/oozie/util/ZKUtils.java 56055b8
core/src/test/java/org/apache/oozie/service/TestDistributedUUIDService.java e69de29
core/src/test/java/org/apache/oozie/service/TestZKUUIDService.java e69de29
Diff: https://reviews.apache.org/r/21046/diff/
Testing
-------
UTC
Thanks,
Purshotam Shah