[
https://issues.apache.org/jira/browse/GOBBLIN-2142?focusedWorklogId=931788&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-931788
]
ASF GitHub Bot logged work on GOBBLIN-2142:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 26/Aug/24 17:22
Start Date: 26/Aug/24 17:22
Worklog Time Spent: 10m
Work Description: phet commented on code in PR #4037:
URL: https://github.com/apache/gobblin/pull/4037#discussion_r1731558718
##########
gobblin-service/src/test/java/org/apache/gobblin/service/modules/orchestration/DagManagerUtilsTest.java:
##########
@@ -209,6 +255,19 @@ public void
testIsDagFinishedWithFinishRunningFailureOption() throws URISyntaxEx
Assert.assertFalse(DagProcUtils.isDagFinished(dag));
}
+ private List<Dag.DagNode<JobExecutionPlan>>
randomizeNodes(Dag<JobExecutionPlan> dag) {
+ int size = dag.getNodes().size();
+ List<Dag.DagNode<JobExecutionPlan>> randomizedDagNodes = new ArrayList<>();
+
+ for (int i=0; i<size; i++) {
+ int index = rand.nextInt(size);
+ randomizedDagNodes.add(dag.getNodes().get(index));
+ }
+
+ return randomizedDagNodes;
+
+ }
Review Comment:
since this is the only place `rand` is used, suggest to make that a local
var to the method.
##########
gobblin-service/src/test/java/org/apache/gobblin/service/modules/orchestration/DagManagerUtilsTest.java:
##########
@@ -263,6 +333,8 @@ public static Dag<JobExecutionPlan> buildComplexDag1(String
id, long flowExecuti
return new JobExecutionPlanDagFactory().createDag(jobExecutionPlans);
}
+ // This creates a dag like this
+ // D0 -> D1 -> D2 -> D3
public static Dag<JobExecutionPlan> buildComplexDag2(String id, long
flowExecutionId,
Review Comment:
NBD, but I might call `buildLinearDagOf4Nodes`
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/proc/DagProcUtils.java:
##########
@@ -309,52 +311,43 @@ public static boolean isDagFinished(Dag<JobExecutionPlan>
dag) {
ExecutionStatus status = node.getValue().getExecutionStatus();
if (status == ExecutionStatus.FAILED || status ==
ExecutionStatus.CANCELLED) {
anyFailure = true;
- removeChildrenFromCanRun(node, dag, canRun);
+ removeDescendantsFromCanRun(node, dag, canRun);
completed.add(node);
} else if (status == ExecutionStatus.COMPLETE) {
completed.add(node);
} else if (status == ExecutionStatus.PENDING) {
// Remove PENDING node if its parents are not in canRun, this means
remove the pending nodes also from canRun set
// if its parents cannot run
- if (!areParentsInCanRun(node, canRun)) {
+ if (!areAllParentsInCanRun(node, canRun)) {
canRun.remove(node);
}
}
}
- // In the end, check if there are more nodes in canRun than completed
assert canRun.size() >= completed.size();
if (!anyFailure || failureOption ==
DagManager.FailureOption.FINISH_ALL_POSSIBLE) {
+ // In the end, check if there are more nodes in canRun than completed
return canRun.size() == completed.size();
} else if (failureOption == DagManager.FailureOption.FINISH_RUNNING) {
- //if all the remaining jobs are pending return true
+ // if all the remaining jobs are pending return true
Review Comment:
this comment doesn't seem to match...
isn't it more like
```
// if NO remaining jobs are pending, return IS finished
```
?
Issue Time Tracking
-------------------
Worklog Id: (was: 931788)
Time Spent: 2h 20m (was: 2h 10m)
> find if the dag is running or not correctly
> -------------------------------------------
>
> Key: GOBBLIN-2142
> URL: https://issues.apache.org/jira/browse/GOBBLIN-2142
> Project: Apache Gobblin
> Issue Type: Bug
> Reporter: Arjun Singh Bora
> Priority: Major
> Time Spent: 2h 20m
> Remaining Estimate: 0h
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
