You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Ryota Egashira <eg...@yahoo-inc.com> on 2014/10/28 18:44:08 UTC
Review Request 27291: OOZIE-1293 Persistent implementation of Events
Queue
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27291/
-----------------------------------------------------------
Review request for oozie.
Bugs: OOZIE-1293
https://issues.apache.org/jira/browse/OOZIE-1293
Repository: oozie-git
Description
-------
https://issues.apache.org/jira/browse/OOZIE-1293
Diffs
-----
client/pom.xml 365276d
client/src/main/java/org/apache/oozie/client/event/Event.java 2ec80f7
client/src/main/java/org/apache/oozie/client/event/JobEvent.java bf7b116
core/src/main/java/org/apache/oozie/ErrorCode.java 4afeb6c
core/src/main/java/org/apache/oozie/event/BundleJobEvent.java b31f0db
core/src/main/java/org/apache/oozie/event/CoordinatorActionEvent.java cd3129f
core/src/main/java/org/apache/oozie/event/CoordinatorJobEvent.java 833a8f7
core/src/main/java/org/apache/oozie/event/EventQueue.java 9ef6dfa
core/src/main/java/org/apache/oozie/event/MemoryEventQueue.java 205dbb6
core/src/main/java/org/apache/oozie/event/WorkflowActionEvent.java c5a38ad
core/src/main/java/org/apache/oozie/event/WorkflowJobEvent.java 3e88afd
core/src/main/java/org/apache/oozie/executor/jpa/sla/SLARegistrationReadOnRestartJPAExecutor.java PRE-CREATION
core/src/main/java/org/apache/oozie/service/EventHandlerService.java 244c048
core/src/main/java/org/apache/oozie/sla/SLACalcStatus.java 189d5ea
core/src/test/java/org/apache/oozie/event/TestBundleJobEvent.java PRE-CREATION
core/src/test/java/org/apache/oozie/event/TestCoordinatorActionEvent.java PRE-CREATION
core/src/test/java/org/apache/oozie/event/TestCoordinatorJobEvent.java PRE-CREATION
core/src/test/java/org/apache/oozie/event/TestEventQueue.java 3bb6c56
core/src/test/java/org/apache/oozie/event/TestMemoryEventQueue.java PRE-CREATION
core/src/test/java/org/apache/oozie/event/TestSLACalcStatus.java PRE-CREATION
core/src/test/java/org/apache/oozie/event/TestWorkflowActionEvent.java PRE-CREATION
core/src/test/java/org/apache/oozie/event/TestWorkflowJobEvent.java PRE-CREATION
core/src/test/java/org/apache/oozie/service/TestEventHandlerService.java 4c2010b
Diff: https://reviews.apache.org/r/27291/diff/
Testing
-------
upload for early review and discussion, still doing evaluation
Thanks,
Ryota Egashira
Re: Review Request 27291: OOZIE-1293 Persistent implementation of
Events Queue
Posted by Purshotam Shah <pu...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27291/#review59064
-----------------------------------------------------------
Over all few comments.
1. Can we support writing to multiple files. We can support writing to local disk and HDFS.
Reading form local files system will be much faster, than reading form HDFS.
We should first read/write to local file and back to HDFS.
On start-up we should find latest path form all QUEUE_PERSIST_PATH in order and which ever is latest we should read form there.
If all path from QUEUE_PERSIST_PATH has latest path, then we should read form path which is first in order.
We should also document that admin should configure local file first and then DFS that, because reading and writing will happen in order.
2. What about delete?
Once persist queue is loaded we should delete it. This is going to be tricky, we should not delete unprocessed or in-process persist queue.
3. In HA what if one server goes down, it may not come up in few hours (or days bcz of ACL or env issue). Other server should processed that persist queue from other server.
This can be done by setting zookeeper watchers.
4. We should also take care of partial write.
Assume there are two servers in HA and one goes down and comes it's coming up. Other is in process of going down. The first server should not read the partial persist event queue from second server.
client/pom.xml
<https://reviews.apache.org/r/27291/#comment100338>
You can remove this.
core/src/main/java/org/apache/oozie/event/BundleJobEvent.java
<https://reviews.apache.org/r/27291/#comment100652>
Should be long
core/src/main/java/org/apache/oozie/event/CoordinatorActionEvent.java
<https://reviews.apache.org/r/27291/#comment100674>
JobEvent has following fileds.
private String id;
private String parentId;
private String user;
private AppType appType;
private String appName;
private EventStatus eventStatus;
private Date startTime;
private Date endTime;
Can you have logic like this?
public void write(DataOutput dataOutput) throws IOException {
dataOutput.writeInt(serialVersionUID);
super.write(dataOutput);
then wtite class specific variable.
In that way you don't have to care about reading and writing parent class variables.
core/src/main/java/org/apache/oozie/event/MemoryEventQueue.java
<https://reviews.apache.org/r/27291/#comment100675>
>Persistent local path is not specified
can be a hdfs path.
core/src/main/java/org/apache/oozie/event/MemoryEventQueue.java
<https://reviews.apache.org/r/27291/#comment100684>
What if HDFS is down?
core/src/main/java/org/apache/oozie/event/MemoryEventQueue.java
<https://reviews.apache.org/r/27291/#comment100682>
copy paste?
core/src/main/java/org/apache/oozie/event/MemoryEventQueue.java
<https://reviews.apache.org/r/27291/#comment100683>
Can you move ShareLibService.getLatestLibPath() to a utility class, so that we don't have duplicate code.
core/src/main/java/org/apache/oozie/sla/SLACalcStatus.java
<https://reviews.apache.org/r/27291/#comment100690>
Long
core/src/main/java/org/apache/oozie/sla/SLACalcStatus.java
<https://reviews.apache.org/r/27291/#comment100691>
Why does event have SLARegistrationBean, It should have only those field which are part of event.
Logically you should be able to just serialized deserialized the class.
- Purshotam Shah
On Oct. 28, 2014, 5:44 p.m., Ryota Egashira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27291/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2014, 5:44 p.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-1293
> https://issues.apache.org/jira/browse/OOZIE-1293
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/OOZIE-1293
>
>
> Diffs
> -----
>
> client/pom.xml 365276d
> client/src/main/java/org/apache/oozie/client/event/Event.java 2ec80f7
> client/src/main/java/org/apache/oozie/client/event/JobEvent.java bf7b116
> core/src/main/java/org/apache/oozie/ErrorCode.java 4afeb6c
> core/src/main/java/org/apache/oozie/event/BundleJobEvent.java b31f0db
> core/src/main/java/org/apache/oozie/event/CoordinatorActionEvent.java cd3129f
> core/src/main/java/org/apache/oozie/event/CoordinatorJobEvent.java 833a8f7
> core/src/main/java/org/apache/oozie/event/EventQueue.java 9ef6dfa
> core/src/main/java/org/apache/oozie/event/MemoryEventQueue.java 205dbb6
> core/src/main/java/org/apache/oozie/event/WorkflowActionEvent.java c5a38ad
> core/src/main/java/org/apache/oozie/event/WorkflowJobEvent.java 3e88afd
> core/src/main/java/org/apache/oozie/executor/jpa/sla/SLARegistrationReadOnRestartJPAExecutor.java PRE-CREATION
> core/src/main/java/org/apache/oozie/service/EventHandlerService.java 244c048
> core/src/main/java/org/apache/oozie/sla/SLACalcStatus.java 189d5ea
> core/src/test/java/org/apache/oozie/event/TestBundleJobEvent.java PRE-CREATION
> core/src/test/java/org/apache/oozie/event/TestCoordinatorActionEvent.java PRE-CREATION
> core/src/test/java/org/apache/oozie/event/TestCoordinatorJobEvent.java PRE-CREATION
> core/src/test/java/org/apache/oozie/event/TestEventQueue.java 3bb6c56
> core/src/test/java/org/apache/oozie/event/TestMemoryEventQueue.java PRE-CREATION
> core/src/test/java/org/apache/oozie/event/TestSLACalcStatus.java PRE-CREATION
> core/src/test/java/org/apache/oozie/event/TestWorkflowActionEvent.java PRE-CREATION
> core/src/test/java/org/apache/oozie/event/TestWorkflowJobEvent.java PRE-CREATION
> core/src/test/java/org/apache/oozie/service/TestEventHandlerService.java 4c2010b
>
> Diff: https://reviews.apache.org/r/27291/diff/
>
>
> Testing
> -------
>
> upload for early review and discussion, still doing evaluation
>
>
> Thanks,
>
> Ryota Egashira
>
>