[
https://issues.apache.org/jira/browse/GOBBLIN-2107?focusedWorklogId=925138&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-925138
]
ASF GitHub Bot logged work on GOBBLIN-2107:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 09/Jul/24 18:01
Start Date: 09/Jul/24 18:01
Worklog Time Spent: 10m
Work Description: phet commented on code in PR #3996:
URL: https://github.com/apache/gobblin/pull/3996#discussion_r1670930350
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/Orchestrator.java:
##########
@@ -289,7 +293,7 @@ public void submitFlowToDagManager(FlowSpec flowSpec,
Dag<JobExecutionPlan> jobE
Note that the responsibility of the multi-active scheduler mode ends
after this method is completed AND the
consumption of a launch type event is committed to the consumer.
*/
- this.dagManager.addDag(jobExecutionPlanDag, true, true);
+ this.dagManager.addDagAndRemoveAdhocFlowSpec(flowSpec,
jobExecutionPlanDag, true, true);
Review Comment:
now the attempt to remove an ad hoc spec would occur twice, since it's also
on line 267.
I believe your aim is to have removal also happen in the path from
[DagActionStoreChangeMonitor](https://github.com/apache/gobblin/blob/4721d086f78553cab28e46bf406787df6cc2dea8/gobblin-service/src/main/java/org/apache/gobblin/service/monitoring/DagActionStoreChangeMonitor.java#L318),
correct?
couldn't that run into the same issue, where if the flow compilation
validation helper fails (see L281 directly above), so the call to this new
combo addDag+removeAdHoc would be skipped?
this new PR definitely shows me I missed the `DagActionStoreChangeMonitor`
case in my recent PR (sorry to see). the crux there was to disentangle adding
the dag from removing the ad hoc spec. the latter MUST happen, suggesting it
belongs inside a `finally`, whereas the former is only best-effort. I would
imagine you'll want the same approach here, in the change monitor code path I
failed to notice
Issue Time Tracking
-------------------
Worklog Id: (was: 925138)
Time Spent: 50m (was: 40m)
> Delete adhoc flowSpecs from flowCatalog
> ----------------------------------------
>
> Key: GOBBLIN-2107
> URL: https://issues.apache.org/jira/browse/GOBBLIN-2107
> Project: Apache Gobblin
> Issue Type: Bug
> Components: gobblin-service
> Reporter: Urmi Mustafi
> Assignee: Abhishek Tiwari
> Priority: Major
> Time Spent: 50m
> Remaining Estimate: 0h
>
> Delete adhoc flowSpecs from flowCatalog to avoid build up of adhoc flowSpecs
> in catalog even when job compilation fails by adding the deletion in a
> finally block in non-MA scheduler case.
> # Previous PR: [https://github.com/apache/gobblin/pull/3944/files] causes a
> bug where adhoc flows are not deleted when multi-active scheduler is enabled.
> This PR re-adds deletion of flowSpec to the dagManager to be called by the
> active host for the MA scheduler case (there will only be one active host if
> dagManager is enabled otherwise code will use dagProcessingEngine).
> # It also ensures quota is released and failed flow compilation event sent
> by the FlowCompilationValidationHelper in all compilation failure cases. This
> was being incorrectly handled before.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
