You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Robert Kanter <rk...@cloudera.com> on 2015/10/09 02:53:34 UTC
Re: Review Request 38474: OOZIE-1976- Specifying coordinator input
datasets in more logical ways
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38474/#review101982
-----------------------------------------------------------
As a general point, can you add some additional comments/javadoc? Especially for the large new areas of code and the Jexl stuff.
client/src/main/resources/oozie-coordinator-0.5.xsd (line 106)
<https://reviews.apache.org/r/38474/#comment159491>
Trailing whitespace. Also more of these throughout the patch.
client/src/main/resources/oozie-coordinator-0.5.xsd (line 110)
<https://reviews.apache.org/r/38474/#comment159492>
Is there any way to enforce a max depth on nested "and"s and "or"s via the schema?
client/src/main/resources/oozie-coordinator-0.5.xsd (line 132)
<https://reviews.apache.org/r/38474/#comment159514>
xs:int
client/src/main/resources/oozie-coordinator-0.5.xsd (line 137)
<https://reviews.apache.org/r/38474/#comment159515>
xs:int
core/pom.xml (line 373)
<https://reviews.apache.org/r/38474/#comment159493>
Version should be defined in root pom
core/src/main/java/org/apache/oozie/CoordinatorActionBean.java
<https://reviews.apache.org/r/38474/#comment159494>
I'm not sure what this does exactly, but is it okay to delete?
core/src/main/java/org/apache/oozie/action/ActionExecutor.java
<https://reviews.apache.org/r/38474/#comment159498>
"fake" change.
core/src/main/java/org/apache/oozie/coord/dependency/CoordDependenciesInputCheck.java (line 69)
<https://reviews.apache.org/r/38474/#comment159504>
so we don't lose the stack trace:
````
throw new IOException(e.getMessage, e);
````
core/src/main/java/org/apache/oozie/coord/dependency/CoordDependenciesInputCheck.java (line 75)
<https://reviews.apache.org/r/38474/#comment159505>
Is there anyway to reuse this without always creating a new one everywhere? (is it not thread safe?)
core/src/main/java/org/apache/oozie/coord/dependency/CoordDependenciesInputCheck.java (line 120)
<https://reviews.apache.org/r/38474/#comment159506>
Should this be the OozieJexlEngine?
core/src/main/java/org/apache/oozie/coord/dependency/CoordDependenciesInputCheck.java (line 137)
<https://reviews.apache.org/r/38474/#comment159507>
Should this be the OozieJexlEngine?
core/src/main/java/org/apache/oozie/coord/dependency/CoordDependency.java (lines 48 - 50)
<https://reviews.apache.org/r/38474/#comment159508>
Most of the time, these are re-assigned in the constructors. I think we can simpley declare them here instead of making new objects.
core/src/main/java/org/apache/oozie/coord/dependency/CoordDependency.java (line 290)
<https://reviews.apache.org/r/38474/#comment159509>
isDependencyMet()
core/src/main/java/org/apache/oozie/coord/dependency/CoordInputCheckerPhaseOne.java (line 111)
<https://reviews.apache.org/r/38474/#comment159510>
We should probably do something better here
core/src/main/java/org/apache/oozie/coord/dependency/CoordInputCheckerPhaseOne.java (line 180)
<https://reviews.apache.org/r/38474/#comment159511>
We should probably do something better here
core/src/main/java/org/apache/oozie/coord/dependency/CoordInputCheckerPhaseThree.java (line 78)
<https://reviews.apache.org/r/38474/#comment159512>
We should probably do something better here
core/src/main/java/org/apache/oozie/coord/dependency/InputLogicParser.java (line 114)
<https://reviews.apache.org/r/38474/#comment159513>
wait isn't used?
We also need to check that it's a valid int > 0
core/src/main/java/org/apache/oozie/service/SchemaService.java (line 75)
<https://reviews.apache.org/r/38474/#comment159516>
This is no longer in the code; it's now in oozie-default under oozie.service.SchemaService.coord.schemas. You'll probably hit this when you rebase.
- Robert Kanter
On Sept. 18, 2015, 12:20 a.m., Purshotam Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38474/
> -----------------------------------------------------------
>
> (Updated Sept. 18, 2015, 12:20 a.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-1976
> https://issues.apache.org/jira/browse/OOZIE-1976
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> There are three components in this patch
>
> 1. User interface
> A new tag is added to coordinator.xml
> ex.
> <input-check>
> <or name="test">
> <and>
> <data-in dataset="A"/>"
> <data-in dataset="B"/>
> </and>
> <and>
> <data-in dataset="C"/>
> <data-in dataset="D"/>
> </and>
>
> </or>;
> <input-check>
>
>
> input-check will have nested and/or/combine operation. It can have min and wait at operator or at date-in.
> If input-check tag is missing then it consider to be old approach where all data dependency are needed.
>
> 2. Processing
> input-check is converted into logical expression
> (a&&B)||(c&&d)
> We use jexl to parse the logical expression.
>
> There are three phase in parsing.
> phase 1 : only resolved dataset are parsed ( only current).
> phase 2 : once all current are resolved, then future/latest are parsed.
> phase 3 : Doesn't do any filecheck, just return what is being parsed by phase1 and phase2. Is used for EL functions
>
>
> 3. Storage.
> if inputcheck is enable, push_missing_dependencies and missing_dependencies are serialized and stored in DB.
> If then not then it's old approach, where they are stored in plan text. This is backward compatible.
>
>
> Diffs
> -----
>
> client/src/main/resources/oozie-coordinator-0.5.xsd e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/pom.xml ca40e2e22293a3df2841764ce725420857425139
> core/src/main/java/org/apache/oozie/CoordinatorActionBean.java 188b70e2e76858228b4d42e5798952383719a93d
> core/src/main/java/org/apache/oozie/action/ActionExecutor.java ff836fbbbe95ca03aace1136abea9548306b2cf2
> core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java a975f6edd40ef674638d1c32c36d5234b78beb0f
> core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 4e1c5b3392358cb6b1e98e16e469310338f27fed
> core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java ca8175e8b8019a42e2780fba4f8fbd313577494c
> core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 548946f05beacbba032b1f411fdae17ca7dd1f44
> core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java cc346274e2f28fd3eb062e8d3550281486c23954
> core/src/main/java/org/apache/oozie/coord/CoordELEvaluator.java 8a279c06a62bd1e6502104f0760d7a1a9792b4bd
> core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 7f59186bb508536e5662bc3ed25c7ec68ba1e2a7
> core/src/main/java/org/apache/oozie/coord/SyncCoordAction.java 44258eb5be40bf6769e32c8780117e8533d80d7e
> core/src/main/java/org/apache/oozie/coord/dependency/CoordDependenciesInputCheck.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/dependency/CoordDependency.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/dependency/CoordInputCheckerPhaseOne.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/dependency/CoordInputCheckerPhaseOnePush.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/dependency/CoordInputCheckerPhaseThree.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/dependency/CoordInputCheckerPhaseTwo.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/dependency/CoordInputCheckerUtility.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/dependency/CoordInputDependencies.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/dependency/CoordInputInstance.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/dependency/CoordPullDependency.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/dependency/CoordPushDependency.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/dependency/CoordUnResolvedDependency.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/dependency/InputLogicParser.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/dependency/OozieJexlEngine.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/dependency/OozieJexlInterpreter.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/dependency/OozieJexlNode.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/dependency/DependencyChecker.java a5507575a3403103133f85a78c873c9976419857
> core/src/main/java/org/apache/oozie/service/SchemaService.java 32105857f51eea9b2e4fd4d9d8cb74900fcdbac8
> core/src/main/java/org/apache/oozie/util/Pair.java 1bf45b41edf19ad0a3115a7bafc010daef11b5c1
> core/src/main/resources/oozie-default.xml 19cae9d4e07ea1c3cd3b287d23cacb9b342b406e
> core/src/test/java/org/apache/oozie/command/coord/TestCoordActionInputCheckXCommand.java f79c9a06c8238aa17b7d331afcb3f137f0bb83f9
> core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 59b8b48b564b04ddd6eb263f8766bf3a8919a429
> core/src/test/java/org/apache/oozie/coord/dependency/TestCoordInputCheckPush.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/test/java/org/apache/oozie/coord/dependency/TestCoordinatorInputCheck.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/test/java/org/apache/oozie/coord/dependency/TestInputLogicParser.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/test/resources/coord-inputcheck-combine.xml e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/test/resources/coord-inputcheck-hcat.xml e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/test/resources/coord-inputcheck-latest.xml e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/test/resources/coord-inputcheck-range-latest.xml e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/test/resources/coord-inputcheck-range.xml e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/test/resources/coord-inputcheck.xml e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
>
> Diff: https://reviews.apache.org/r/38474/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Purshotam Shah
>
>
Re: Review Request 38474: OOZIE-1976- Specifying coordinator input
datasets in more logical ways
Posted by Purshotam Shah <pu...@yahoo-inc.com>.
> On Oct. 9, 2015, 12:53 a.m., Robert Kanter wrote:
> > client/src/main/resources/oozie-coordinator-0.5.xsd, line 110
> > <https://reviews.apache.org/r/38474/diff/1/?file=1076234#file1076234line110>
> >
> > Is there any way to enforce a max depth on nested "and"s and "or"s via the schema?
Not sure. We are not enforcing anything. if its needed we can do it in future.
> On Oct. 9, 2015, 12:53 a.m., Robert Kanter wrote:
> > core/src/main/java/org/apache/oozie/coord/dependency/CoordDependenciesInputCheck.java, line 75
> > <https://reviews.apache.org/r/38474/diff/1/?file=1076246#file1076246line75>
> >
> > Is there anyway to reuse this without always creating a new one everywhere? (is it not thread safe?)
Haven't checked that, but OozieJexlEngine is very simple.
public class OozieJexlEngine extends JexlEngine {
OozieJexlInterpreter oozieInterpreter;
public OozieJexlEngine() {
}
- Purshotam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38474/#review101982
-----------------------------------------------------------
On Dec. 15, 2015, 7:33 p.m., Purshotam Shah wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38474/
> -----------------------------------------------------------
>
> (Updated Dec. 15, 2015, 7:33 p.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-1976
> https://issues.apache.org/jira/browse/OOZIE-1976
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> There are three components in this patch
>
> 1. User interface
> A new tag is added to coordinator.xml
> ex.
> <input-check>
> <or name="test">
> <and>
> <data-in dataset="A"/>"
> <data-in dataset="B"/>
> </and>
> <and>
> <data-in dataset="C"/>
> <data-in dataset="D"/>
> </and>
>
> </or>;
> <input-check>
>
>
> input-check will have nested and/or/combine operation. It can have min and wait at operator or at date-in.
> If input-check tag is missing then it consider to be old approach where all data dependency are needed.
>
> 2. Processing
> input-check is converted into logical expression
> (a&&B)||(c&&d)
> We use jexl to parse the logical expression.
>
> There are three phase in parsing.
> phase 1 : only resolved dataset are parsed ( only current).
> phase 2 : once all current are resolved, then future/latest are parsed.
> phase 3 : Doesn't do any filecheck, just return what is being parsed by phase1 and phase2. Is used for EL functions
>
>
> 3. Storage.
> if inputcheck is enable, push_missing_dependencies and missing_dependencies are serialized and stored in DB.
> If then not then it's old approach, where they are stored in plan text. This is backward compatible.
>
>
> Diffs
> -----
>
> client/src/main/resources/oozie-coordinator-0.5.xsd e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/pom.xml b063dab79415447a86c1a33f5c3f5304e0dffca0
> core/src/main/java/org/apache/oozie/CoordinatorActionBean.java 91bff4dca2ef2dece68ca7260724ba3e43b1a08e
> core/src/main/java/org/apache/oozie/ErrorCode.java 6c1e3997c9a1cb0bb0de39d687a083af0a7b5f04
> core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java 742d00dd47aab55392abc8fbc207f8100728b832
> core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java 4e1c5b3392358cb6b1e98e16e469310338f27fed
> core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java 58ef483272264e0d391d4a1e6533dc1cab9940da
> core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 39e6ac15ce3a3ea7f2ed9178688537f7b1d7842d
> core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java b05344d89e8df0e11fe69c1aa725d19a18eb0a2b
> core/src/main/java/org/apache/oozie/coord/CoordELConstants.java f010a817fc900821c0e429fc16e1d3902a98d8bb
> core/src/main/java/org/apache/oozie/coord/CoordELEvaluator.java 8b2f4560ae66bbcd707a446382e647663ea67be1
> core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java 5d238663aa94f0dd55a9190b60bfe621439c7b53
> core/src/main/java/org/apache/oozie/coord/CoordUtils.java 94c69740618110ea180b188ab0c5a02db76f8b4d
> core/src/main/java/org/apache/oozie/coord/SyncCoordAction.java 44258eb5be40bf6769e32c8780117e8533d80d7e
> core/src/main/java/org/apache/oozie/coord/input/dependency/AbstractCoordInputDependency.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/input/dependency/CoordInputDependenciesBuilder.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/input/dependency/CoordInputDependenciesChecker.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/input/dependency/CoordInputDependenciesUtil.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/input/dependency/CoordInputDependency.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/input/dependency/CoordInputDependencyCheck.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/input/dependency/CoordInputDependencyCheckPhaseOne.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/input/dependency/CoordInputDependencyCheckPhaseThree.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/input/dependency/CoordInputDependencyCheckPhaseTwo.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/input/dependency/CoordInputDependencyCheckPhaseValidate.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/input/dependency/CoordInputDependencyFactory.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/input/dependency/CoordInputInstance.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/input/dependency/CoordOldInputDependency.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/input/dependency/CoordPullInputDependency.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/input/dependency/CoordPushInputDependency.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/input/dependency/CoordUnResolvedInputDependency.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/input/dependency/InputLogicParser.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/input/dependency/OozieJexlEngine.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/coord/input/dependency/OozieJexlInterpreter.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/main/java/org/apache/oozie/dependency/ActionDependency.java c280d1dc2387c9b3df96d4e83ad5563e10b1e1ef
> core/src/main/java/org/apache/oozie/dependency/DependencyChecker.java a5507575a3403103133f85a78c873c9976419857
> core/src/main/java/org/apache/oozie/util/WritableUtils.java 76a68953541125cab05bd05c94aa73b50e5363ff
> core/src/main/resources/oozie-default.xml faf37405757719554a5b6e3671d0c87723eb9959
> core/src/test/java/org/apache/oozie/command/coord/TestCoordActionInputCheckXCommand.java 1fe1b3adb5ceaa83985ef31278e49a3224cde0fb
> core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java 7062e697e5162ac25688d9cb62352bd139a7013a
> core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java 29e7ca145612fadee661fe975aa0ea2cab1cbfa3
> core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java 52eb9dd1fb903ac86294a55d6e2a6918390bf4e9
> core/src/test/java/org/apache/oozie/coord/input/dependency/TestCoordInputCheckPush.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/test/java/org/apache/oozie/coord/input/dependency/TestCoordinatorInputCheck.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/test/java/org/apache/oozie/coord/input/dependency/TestInputLogicParser.java e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/test/resources/coord-action-sla.xml f3f1bc09c51272a35d65edd7271599e7e8ba1ba4
> core/src/test/resources/coord-dataset-offset.xml 5dfbb1b40ee86dfc3336031eea3c5ff3144b9f5a
> core/src/test/resources/coord-inputcheck-combine.xml e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/test/resources/coord-inputcheck-hcat.xml e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/test/resources/coord-inputcheck-latest.xml e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/test/resources/coord-inputcheck-range-latest.xml e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/test/resources/coord-inputcheck-range.xml e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> core/src/test/resources/coord-inputcheck.xml e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> pom.xml a74ffab081defd15a1c5dbbf15355aa4e0455029
>
> Diff: https://reviews.apache.org/r/38474/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Purshotam Shah
>
>