umustafi commented on code in PR #3661:
URL: https://github.com/apache/gobblin/pull/3661#discussion_r1175638739
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/DagManager.java:
##########
@@ -681,18 +681,20 @@ private void cancelDag(DagId dagId) throws
ExecutionException, InterruptedExcept
clearUpDagAction(dagId);
}
- private void cancelDagNode(DagNode<JobExecutionPlan> dagNodeToCancel)
throws ExecutionException, InterruptedException {
+ private void cancelDagNode(DagNode<JobExecutionPlan> dagNodeToCancel,
boolean shouldSendCancellationEvent) throws ExecutionException,
InterruptedException {
Review Comment:
can you add a comment to explain why you added this boolean and what case we
don't send cancellation event even though this function is called?
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/DagManager.java:
##########
@@ -853,27 +852,40 @@ private boolean
killJobIfOrphaned(DagNode<JobExecutionPlan> node, JobStatus jobS
DagManagerUtils.getFullyQualifiedDagName(node),
timeOutForJobStart);
dagManagerMetrics.incrementCountsStartSlaExceeded(node);
- cancelDagNode(node);
String dagId = DagManagerUtils.generateDagId(node).toString();
-
this.dags.get(dagId).setFlowEvent(TimingEvent.FlowTimings.FLOW_START_DEADLINE_EXCEEDED);
+
this.dags.get(dagId).setFlowEvent(TimingEvent.FlowTimings.FLOW_CANCELLED);
Review Comment:
we're not calling `cancelDagNode` but still updating flow status also
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
