-----------------------------------------------------------
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
> 
>

Reply via email to