You are viewing a plain text version of this content. The canonical link for it is here.

Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/04/19 14:25:13 UTC

[GitHub] [beam] jrmccluskey commented on a diff in pull request #17386: [BEAM-11104] Add Checkpointing split to Go SDK

jrmccluskey commented on code in PR #17386:
URL: https://github.com/apache/beam/pull/17386#discussion_r853141017


##########
sdks/go/pkg/beam/core/runtime/harness/harness.go:
##########
@@ -401,11 +402,34 @@ func (c *control) handleInstruction(ctx context.Context, req *fnpb.InstructionRe
 				c.plans[bdID] = append(c.plans[bdID], plan)
 			}
 		}
+
+		// Check if the underlying DoFn self-checkpointed.
+		// TODO(BEAM-11104): How should a returned error here be handled to avoid clobbering
+		// an error that is returned after the mutex is given up?

Review Comment:
   Yeah I'm thinking the original error would be more important than a checkpoint failing. I was trying to decide if it was important to include both, but it makes sense that the first error should be prioritized. 



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org