Skip to content

Commit 29e4dfe

Browse files
committed
Fix deadlock on worker failures by introducing a bundle-level Done signal
The cancellation signal `b.Done` is to abort bundle data streaming immediately if the worker fails or completes the bundle early. Without this signal, a deadlock scenario can occur. - Runner sends elements through data channel - SDK worker reads and processes the first element, raises an exception and sends InstructionResponse through control channel. However, at the same time, Runner keeps sending data and is blocked until SDK worker reads more data. - The existed two signals are not sufficient to do an immediate break - ctx.Done() is closed when the pipeline is done or timeout. Given the bundle failure, timeout is the only option but it is too late. - wk.StoppedChan() is closed when the worker is stopped by the worker pool, which is also not happening when the runner is waiting to send data.
1 parent 0aa1359 commit 29e4dfe

2 files changed

Lines changed: 86 additions & 0 deletions

File tree

sdks/go/pkg/beam/runners/prism/internal/worker/bundle.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ type B struct {
6262
OutputData engine.TentativeData
6363

6464
Resp chan *fnpb.ProcessBundleResponse
65+
Done chan struct{}
6566
BundleErr error
6667
responded bool
6768

@@ -80,6 +81,7 @@ func (b *B) Init() {
8081
close(b.DataWait) // Can happen if there are no outputs for the bundle.
8182
}
8283
b.Resp = make(chan *fnpb.ProcessBundleResponse, 1)
84+
b.Done = make(chan struct{})
8385
}
8486

8587
// DataOrTimerDone indicates a final element has been received from a Data or Timer output.
@@ -102,7 +104,11 @@ func (b *B) Respond(resp *fnpb.InstructionResponse) {
102104
return
103105
}
104106
b.responded = true
107+
if b.Done != nil {
108+
close(b.Done)
109+
}
105110
if resp.GetError() != "" {
111+
slog.Error("DEBUG: Prism received bundle error from worker response", "bundle", resp.GetInstructionId())
106112
b.BundleErr = fmt.Errorf("bundle %v %v failed:%v", resp.GetInstructionId(), b.PBDID, resp.GetError())
107113
close(b.Resp)
108114
return
@@ -143,6 +149,13 @@ func (b *B) ProcessOn(ctx context.Context, wk *W) <-chan struct{} {
143149
b.DataOrTimerDone()
144150
}
145151
return b.DataWait
152+
case <-b.Done:
153+
// The bundle completed/failed before req was sent.
154+
outCap := b.OutputCount + len(b.HasTimers)
155+
for i := 0; i < outCap; i++ {
156+
b.DataOrTimerDone()
157+
}
158+
return b.DataWait
146159
case wk.InstReqs <- req:
147160
// desired outcome
148161
}
@@ -181,6 +194,9 @@ func (b *B) ProcessOn(ctx context.Context, wk *W) <-chan struct{} {
181194
case <-ctx.Done():
182195
b.DataOrTimerDone()
183196
return b.DataWait
197+
case <-b.Done:
198+
b.DataOrTimerDone()
199+
return b.DataWait
184200
case wk.DataReqs <- elms:
185201
}
186202
}
@@ -202,6 +218,9 @@ func (b *B) ProcessOn(ctx context.Context, wk *W) <-chan struct{} {
202218
case <-ctx.Done():
203219
b.DataOrTimerDone()
204220
return b.DataWait
221+
case <-b.Done:
222+
b.DataOrTimerDone()
223+
return b.DataWait
205224
case wk.DataReqs <- &fnpb.Elements{
206225
Timers: timers,
207226
Data: []*fnpb.Elements_Data{

sdks/go/pkg/beam/runners/prism/internal/worker/worker_test.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,73 @@ func TestWorker_State_MultimapSideInput(t *testing.T) {
522522
}
523523
}
524524

525+
// TestBundle_ProcessOn_WorkerFailure verifies that the runner does not deadlock when
526+
// a worker fails mid-bundle and stops reading elements from the Data plane stream.
527+
func TestBundle_ProcessOn_WorkerFailure(t *testing.T) {
528+
ctx, wk, clientConn := serveTestWorker(t)
529+
530+
dataCli := fnpb.NewBeamFnDataClient(clientConn)
531+
dataStream, err := dataCli.Data(ctx)
532+
if err != nil {
533+
t.Fatal("couldn't create data client:", err)
534+
}
535+
536+
instID := wk.NextInst()
537+
538+
// Create 15 large input blocks (512 KB each) to saturate the 10-slot channel buffer
539+
// and the gRPC flow control window, forcing the Data sender inside worker.go to block.
540+
largeBytes := make([]byte, 512*1024)
541+
var inputBlocks []*engine.Block
542+
for i := 0; i < 15; i++ {
543+
inputBlocks = append(inputBlocks, &engine.Block{
544+
Kind: engine.BlockData,
545+
Bytes: [][]byte{largeBytes},
546+
})
547+
}
548+
549+
b := &B{
550+
InstID: instID,
551+
PBDID: "teststageID",
552+
Input: inputBlocks,
553+
OutputCount: 1,
554+
}
555+
b.Init()
556+
wk.activeInstructions[instID] = b
557+
558+
processOnDone := make(chan struct{})
559+
go func() {
560+
b.ProcessOn(ctx, wk)
561+
close(processOnDone)
562+
}()
563+
564+
// Send the initial process bundle request trigger.
565+
wk.InstReqs <- &fnpb.InstructionRequest{
566+
InstructionId: instID,
567+
}
568+
569+
// Read only the first block to simulate worker processing start.
570+
_, err = dataStream.Recv()
571+
if err != nil {
572+
t.Fatal("couldn't receive first data element:", err)
573+
}
574+
575+
// Simulate worker failure by responding with an error on the Control channel.
576+
// Without the fix, ProcessOn's background goroutine deadlocks at `wk.DataReqs <- elms`
577+
// because the client stopped reading and the buffer/flow-control is saturated.
578+
wk.activeInstructions[instID].Respond(&fnpb.InstructionResponse{
579+
InstructionId: instID,
580+
Error: "Intentional worker failure",
581+
})
582+
583+
// Verify that ProcessOn exits cleanly and does not deadlock/hang.
584+
select {
585+
case <-processOnDone:
586+
// Test passed: ProcessOn exited successfully!
587+
case <-time.After(10 * time.Second):
588+
t.Fatal("ProcessOn deadlocked / hung after worker failure!")
589+
}
590+
}
591+
525592
func newWorker() *W {
526593
mw := &MultiplexW{
527594
pool: map[string]*W{},

0 commit comments

Comments
 (0)