[ 
https://issues.apache.org/jira/browse/GOBBLIN-2175?focusedWorklogId=946917&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-946917
 ]

ASF GitHub Bot logged work on GOBBLIN-2175:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 05/Dec/24 17:52
            Start Date: 05/Dec/24 17:52
    Worklog Time Spent: 10m 
      Work Description: phet commented on code in PR #4078:
URL: https://github.com/apache/gobblin/pull/4078#discussion_r1871817070


##########
gobblin-temporal/src/main/java/org/apache/gobblin/temporal/ddm/workflow/impl/ProcessWorkUnitsWorkflowImpl.java:
##########
@@ -72,20 +73,33 @@ private CommitStats performWork(WUProcessingSpec workSpec) {
     searchAttributes = 
TemporalWorkFlowUtils.generateGaasSearchAttributes(jobState.getProperties());
 
     NestingExecWorkflow<WorkUnitClaimCheck> processingWorkflow = 
createProcessingWorkflow(workSpec, searchAttributes);
-    int workunitsProcessed =
-        processingWorkflow.performWorkload(WorkflowAddr.ROOT, workload, 0, 
workSpec.getTuning().getMaxBranchesPerTree(),
-            workSpec.getTuning().getMaxSubTreesPerTree(), Optional.empty());
-    if (workunitsProcessed > 0) {
-      CommitStepWorkflow commitWorkflow = 
createCommitStepWorkflow(searchAttributes);
-      CommitStats result = commitWorkflow.commit(workSpec);
-      if (result.getNumCommittedWorkUnits() == 0) {
-        log.warn("No work units committed at the job level. They could have 
been committed at the task level.");
-      }
-      return result;
-    } else {
+
+    Optional<Integer> workunitsProcessed = Optional.empty();
+    try {
+      workunitsProcessed = 
Optional.of(processingWorkflow.performWorkload(WorkflowAddr.ROOT, workload, 0,
+          workSpec.getTuning().getMaxBranchesPerTree(), 
workSpec.getTuning().getMaxSubTreesPerTree(),
+          Optional.empty()));
+    } catch (Exception e) {
+      log.error("Exception occurred in performing workload,proceeding with 
commit step", e);
+      performCommitIfAnyWorkUnitsProcessed(workSpec, searchAttributes, 
workunitsProcessed);
+      throw e;//We want to proceed with partial commit and throw exception so 
that the parent workflow ExecuteGobblinWorkflowImpl can throw the failure event

Review Comment:
   nit: needs whitespace - after comma in `log.error` and around `//` comment



##########
gobblin-temporal/src/main/java/org/apache/gobblin/temporal/ddm/workflow/impl/ProcessWorkUnitsWorkflowImpl.java:
##########
@@ -72,20 +73,33 @@ private CommitStats performWork(WUProcessingSpec workSpec) {
     searchAttributes = 
TemporalWorkFlowUtils.generateGaasSearchAttributes(jobState.getProperties());
 
     NestingExecWorkflow<WorkUnitClaimCheck> processingWorkflow = 
createProcessingWorkflow(workSpec, searchAttributes);
-    int workunitsProcessed =
-        processingWorkflow.performWorkload(WorkflowAddr.ROOT, workload, 0, 
workSpec.getTuning().getMaxBranchesPerTree(),
-            workSpec.getTuning().getMaxSubTreesPerTree(), Optional.empty());
-    if (workunitsProcessed > 0) {
-      CommitStepWorkflow commitWorkflow = 
createCommitStepWorkflow(searchAttributes);
-      CommitStats result = commitWorkflow.commit(workSpec);
-      if (result.getNumCommittedWorkUnits() == 0) {
-        log.warn("No work units committed at the job level. They could have 
been committed at the task level.");
-      }
-      return result;
-    } else {
+
+    Optional<Integer> workunitsProcessed = Optional.empty();
+    try {
+      workunitsProcessed = 
Optional.of(processingWorkflow.performWorkload(WorkflowAddr.ROOT, workload, 0,
+          workSpec.getTuning().getMaxBranchesPerTree(), 
workSpec.getTuning().getMaxSubTreesPerTree(),
+          Optional.empty()));
+    } catch (Exception e) {
+      log.error("Exception occurred in performing workload,proceeding with 
commit step", e);

Review Comment:
   since the log msg is what's externally observable, it would be much clearer 
to explicitly name "partial commit" as what we're doing.  (e.g. helpful when 
searching through logs).
   
   e.g. `"ProcessWorkUnits failure - will attempt partial commit before 
announcing error"`





Issue Time Tracking
-------------------

    Worklog Id:     (was: 946917)
    Time Spent: 1h 20m  (was: 1h 10m)

> Fix partial commit in temporal flow
> -----------------------------------
>
>                 Key: GOBBLIN-2175
>                 URL: https://issues.apache.org/jira/browse/GOBBLIN-2175
>             Project: Apache Gobblin
>          Issue Type: Bug
>            Reporter: Aditya Pratap Singh
>            Priority: Major
>          Time Spent: 1h 20m
>  Remaining Estimate: 0h
>
> Fix partial commit in temporal flow



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to