Skip to content

Commit 0aa1359

Browse files
committed
Set b.BundleErr correctly when dynamic split happens.
1 parent c33bc97 commit 0aa1359

3 files changed

Lines changed: 57 additions & 1 deletion

File tree

sdks/go/pkg/beam/runners/prism/internal/execute_test.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -541,6 +541,27 @@ func TestFailureHang(t *testing.T) {
541541
}
542542
}
543543

544+
func TestFailure_SplitError(t *testing.T) {
545+
initRunner(t)
546+
547+
p, s := beam.NewPipelineWithRoot()
548+
configs := beam.Create(s, SourceConfig{NumElements: 100, InitialSplits: 1})
549+
col := beam.ParDo(s, &slowFailSDF{}, configs)
550+
beam.ParDo(s, &int64Check{Name: "sdf_fail", Want: []int{}}, col)
551+
552+
// Set a short timeout so the test fails quickly if the pipeline hangs due to split error handling bug
553+
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
554+
defer cancel()
555+
556+
_, err := executeWithT(ctx, t, p)
557+
if err == nil {
558+
t.Fatalf("expected pipeline failure, but got a success")
559+
}
560+
if want := "intentional split error from tracker"; !strings.Contains(err.Error(), want) {
561+
t.Fatalf("expected pipeline failure with %q, but was %v", want, err)
562+
}
563+
}
564+
544565
func TestRunner_Passert(t *testing.T) {
545566
initRunner(t)
546567
tests := []struct {

sdks/go/pkg/beam/runners/prism/internal/stage.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ progress:
240240
sr, err := b.Split(ctx, wk, 0.5 /* fraction of remainder */, nil /* allowed splits */)
241241
if err != nil {
242242
slog.Warn("SDK Error from split, aborting splits and failing bundle", "bundle", rb, "error", err.Error())
243-
if b.BundleErr != nil {
243+
if b.BundleErr == nil {
244244
b.BundleErr = err
245245
}
246246
return b.BundleErr

sdks/go/pkg/beam/runners/prism/internal/testdofns_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ func init() {
6464
register.Function2x1(combineIntSum)
6565

6666
register.DoFn3x1[*sdf.LockRTracker, SourceConfig, func(int64), error]((*intRangeFn)(nil))
67+
register.DoFn4x1[context.Context, *sdf.LockRTracker, SourceConfig, func(int64), error]((*slowFailSDF)(nil))
6768
register.Emitter1[int64]()
6869
register.Emitter2[int64, int64]()
6970
}
@@ -404,3 +405,37 @@ func (fn *selfCheckpointingDoFn) ProcessElement(rt *sdf.LockRTracker, _ []byte,
404405
}
405406
}
406407
}
408+
409+
type errorSplitTracker struct {
410+
*offsetrange.Tracker
411+
}
412+
413+
func (t *errorSplitTracker) TrySplit(fraction float64) (any, any, error) {
414+
return nil, nil, fmt.Errorf("intentional split error from tracker")
415+
}
416+
417+
type slowFailSDF struct{}
418+
419+
func (fn *slowFailSDF) CreateInitialRestriction(config SourceConfig) offsetrange.Restriction {
420+
return offsetrange.Restriction{Start: 0, End: config.NumElements}
421+
}
422+
423+
func (fn *slowFailSDF) SplitRestriction(config SourceConfig, rest offsetrange.Restriction) []offsetrange.Restriction {
424+
return rest.EvenSplits(config.InitialSplits)
425+
}
426+
427+
func (fn *slowFailSDF) RestrictionSize(_ SourceConfig, rest offsetrange.Restriction) float64 {
428+
return rest.Size()
429+
}
430+
431+
func (fn *slowFailSDF) CreateTracker(rest offsetrange.Restriction) *sdf.LockRTracker {
432+
return sdf.NewLockRTracker(&errorSplitTracker{Tracker: offsetrange.NewTracker(rest)})
433+
}
434+
435+
func (fn *slowFailSDF) ProcessElement(ctx context.Context, rt *sdf.LockRTracker, config SourceConfig, emit func(int64)) error {
436+
fmt.Println("DEBUG: slowFailSDF.ProcessElement invoked")
437+
for i := rt.GetRestriction().(offsetrange.Restriction).Start; rt.TryClaim(i); i++ {
438+
<-ctx.Done()
439+
}
440+
return nil
441+
}

0 commit comments

Comments
 (0)