lostluck commented on code in PR #35337:
URL: https://github.com/apache/beam/pull/35337#discussion_r2153092104
##########
sdks/go/pkg/beam/runners/prism/internal/stage.go:
##########
@@ -208,8 +208,8 @@ progress:
ticked = true
resp, err := b.Progress(ctx, wk)
if err != nil {
- slog.Debug("SDK Error from progress, aborting
progress", "bundle", rb, "error", err.Error())
- break progress
+ slog.Debug("SDK Error from progress request,
aborting progress update", "bundle", rb, "error", err.Error())
+ continue progress
Review Comment:
Discussed with Danny, and he agreed that we should stop the ticker here at
least in this CL. Principle of least change by still stopping the progress spam
in this case, but fixing the specific issue (the ignoring the returned bundle
value).
This stop was added when the Swift SDK was being developed, and the progress
requests were causing much spam logging SDK side since Progress wasn't yet
implemented there.
##########
sdks/go/pkg/beam/runners/prism/internal/stage.go:
##########
@@ -224,8 +224,11 @@ progress:
slog.Debug("splitting report", "bundle", rb,
"index", index)
sr, err := b.Split(ctx, wk, 0.5 /* fraction of
remainder */, nil /* allowed splits */)
if err != nil {
- slog.Warn("SDK Error from split,
aborting splits", "bundle", rb, "error", err.Error())
- break progress
+ slog.Warn("SDK Error from split,
aborting splits and failing bundle", "bundle", rb, "error", err.Error())
+ if b.BundleErr != nil {
+ b.BundleErr = err
+ }
+ return b.BundleErr
Review Comment:
Discussed this with Danny, and I concede that there's a chance of data loss
if a split request is sent out, and the SDK applies the split, but something
goes wrong with the split response. The SDK may end up processing less than the
runner thinks it will be processing (due to the error return).
I'm not personally sure this is entirely likely (as an error on return
indicates something else will have gone wrong), but it does seem possible. We
may need to revisit this if it turns out to be a source of different flakes.
No need to change this part of the commit as a result.
--
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]
