Skip to content

Commit feddf87

Browse files
committed
[Prism] Schedule consumers of a self checkpointing source
Fixes #39446. An unbounded source SDF that returns a process continuation residual has that residual re-queued as a pending element carrying the input element's event time, MinTimestamp for an Impulse rooted source, so the stage's watermark never advances. updateWatermarks returns no refreshes when the output watermark does not move, and PersistBundle marked only the producing stage as changed, so a consumer that was just handed pending elements was never surfaced to the scheduler. Its elements accumulated forever. checkForQuiescence cannot catch this, because the source stays schedulable, so the job live locks instead of failing fast. PersistBundle now records the consumers that accepted data, and updateWatermarks hands them to the scheduler on the path where the output watermark does not advance. Recording is gated on any bundle having returned a residual, so a pipeline that never self checkpoints keeps its previous bundle scheduling. bundleReady additionally lets a stateful stage with no side inputs run on pending data alone, under the same gate, since statefulStageKind.buildEventTimeBundle takes data at any watermark and gates only timers. Its stillSchedulable now requires buildable work, and a key that supplies nothing no longer consumes the OneKeyPerBundle slot, is not marked in progress, and does not hold the bundle's minimum timestamp. A consumer that reads a side input still waits on the watermark, since side input readiness is derived from it.
1 parent 42c693f commit feddf87

2 files changed

Lines changed: 377 additions & 13 deletions

File tree

sdks/go/pkg/beam/runners/prism/internal/engine/elementmanager.go

Lines changed: 78 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,10 @@ type ElementManager struct {
235235
livePending atomic.Int64 // An accessible live pending count. DEBUG USE ONLY
236236
pendingElements sync.WaitGroup // pendingElements counts all unprocessed elements in a job. Jobs with no pending elements terminate successfully.
237237

238+
// Latched once any bundle returns a residual, after which a stage watermark
239+
// can be pinned indefinitely and can't be relied on to schedule consumers.
240+
sawResidual atomic.Bool
241+
238242
processTimeEvents *stageRefreshQueue // Manages sequence of stage updates when interfacing with processing time. Callers must hold refreshCond.L lock.
239243
testStreamHandler *testStreamHandler // Optional test stream handler when a test stream is in the pipeline.
240244
}
@@ -841,6 +845,17 @@ func reElementResiduals(residuals []Residual, inputInfo PColInfo, rb RunBundle)
841845
// input elements, and the committed output elements.
842846
func (em *ElementManager) PersistBundle(rb RunBundle, col2Coders map[string]PColInfo, d TentativeData, inputInfo PColInfo, residuals Residuals) {
843847
stage := em.stages[rb.StageID]
848+
// Consumers that received data from this bundle, recorded so they can still
849+
// be scheduled when this stage's output watermark is held back. Only needed
850+
// once something self checkpoints, so pipelines that never do keep their
851+
// previous bundle scheduling exactly.
852+
if len(residuals.Data) > 0 {
853+
em.sawResidual.Store(true)
854+
}
855+
var changedConsumers set[string]
856+
if em.sawResidual.Load() {
857+
changedConsumers = set[string]{}
858+
}
844859
var seq int
845860
for output, data := range d.Raw {
846861
info := col2Coders[output]
@@ -910,6 +925,9 @@ func (em *ElementManager) PersistBundle(rb RunBundle, col2Coders map[string]PCol
910925
count = consumer.AddPending(em, newPending)
911926
}
912927
em.addPending(count)
928+
if changedConsumers != nil && count > 0 {
929+
changedConsumers.insert(sID)
930+
}
913931
}
914932
for _, link := range sideConsumers {
915933
consumer := em.stages[link.Global]
@@ -942,6 +960,12 @@ func (em *ElementManager) PersistBundle(rb RunBundle, col2Coders map[string]PCol
942960
// even if a panic occurs during `em.addPending`. This prevents potential deadlocks
943961
// if the waitgroup unexpectedly drops below zero due to a runner bug.
944962
defer stage.mu.Unlock()
963+
if len(changedConsumers) > 0 {
964+
if stage.consumersWithNewData == nil {
965+
stage.consumersWithNewData = set[string]{}
966+
}
967+
stage.consumersWithNewData.merge(changedConsumers)
968+
}
945969
completed := stage.inprogress[rb.BundleID]
946970
em.addPending(-len(completed.es))
947971
delete(stage.inprogress, rb.BundleID)
@@ -1101,6 +1125,9 @@ func (em *ElementManager) ReturnResiduals(rb RunBundle, firstRsIndex int, inputI
11011125
stage := em.stages[rb.StageID]
11021126

11031127
stage.splitBundle(rb, firstRsIndex, em)
1128+
if len(residuals.Data) > 0 {
1129+
em.sawResidual.Store(true)
1130+
}
11041131
unprocessedElements := reElementResiduals(residuals.Data, inputInfo, rb)
11051132
if len(unprocessedElements) > 0 {
11061133
slog.Debug("ReturnResiduals: unprocessed elements", "bundle", rb, "count", len(unprocessedElements))
@@ -1211,6 +1238,9 @@ type stageState struct {
12111238
estimatedOutput mtime.Time // Estimated watermark output from DoFns
12121239
previousInput mtime.Time // input watermark before the latest watermark refresh
12131240

1241+
// Consumers handed data by a bundle, pending delivery to the scheduler.
1242+
consumersWithNewData set[string]
1243+
12141244
pending elementHeap // pending input elements for this stage that are to be processesd
12151245
inprogress map[string]elements // inprogress elements by active bundles, keyed by bundle
12161246
sideInputs map[LinkID]map[typex.Window][][]byte // side input data for this stage, from {tid, inputID} -> window
@@ -1822,12 +1852,6 @@ keysPerBundle:
18221852
if ss.inprogressKeys.present(k) {
18231853
continue
18241854
}
1825-
newKeys.insert(k)
1826-
// Track the min-timestamp for later watermark handling.
1827-
if dnt.elements[0].timestamp < minTs {
1828-
minTs = dnt.elements[0].timestamp
1829-
}
1830-
18311855
dataInBundle := false
18321856

18331857
var toProcessForKey []element
@@ -1874,22 +1898,51 @@ keysPerBundle:
18741898
break
18751899
}
18761900
}
1901+
if len(toProcessForKey) > 0 {
1902+
newKeys.insert(k)
1903+
// Track the min-timestamp for later watermark handling. Elements pop
1904+
// in timestamp order, so the first selected one is the earliest.
1905+
if ts := toProcessForKey[0].timestamp; ts < minTs {
1906+
minTs = ts
1907+
}
1908+
}
18771909
toProcess = append(toProcess, toProcessForKey...)
18781910

18791911
if dnt.elements.Len() == 0 {
18801912
delete(ss.pendingByKeys, k)
18811913
}
1882-
if OneKeyPerBundle {
1914+
// A key that yielded nothing, such as one headed by a timer above the
1915+
// watermark, must not consume the single key slot, or the bundle is empty
1916+
// and the stage keeps rescheduling on it.
1917+
if OneKeyPerBundle && len(toProcessForKey) > 0 {
18831918
break keysPerBundle
18841919
}
18851920
}
18861921

1887-
// If we're out of data, and timers were not cleared then the watermark is accurate.
1888-
stillSchedulable := !(len(ss.pendingByKeys) == 0 && !timerCleared)
1922+
// Reschedule only when a later bundle could build something, or a cleared
1923+
// timer may have held back the minimum pending timestamp.
1924+
stillSchedulable := timerCleared || ss.hasBuildableDataLocked(watermark)
18891925

18901926
return toProcess, minTs, newKeys, holdsInBundle, nil, stillSchedulable, 0
18911927
}
18921928

1929+
// hasBuildableDataLocked reports whether a key that isn't in progress heads its
1930+
// heap with data, or with a timer the watermark has reached. Callers hold ss.mu.
1931+
func (ss *stageState) hasBuildableDataLocked(watermark mtime.Time) bool {
1932+
for k, dnt := range ss.pendingByKeys {
1933+
if ss.inprogressKeys.present(k) {
1934+
continue
1935+
}
1936+
if dnt.elements.Len() == 0 {
1937+
continue
1938+
}
1939+
if e := dnt.elements[0]; e.IsData() || e.timestamp <= watermark {
1940+
return true
1941+
}
1942+
}
1943+
return false
1944+
}
1945+
18931946
// buildEventTimeBundle for aggregation stages, processes all elements that are within the watermark for completed windows.
18941947
func (*aggregateStageKind) buildEventTimeBundle(ss *stageState, watermark mtime.Time) (toProcess elementHeap, _ mtime.Time, _ set[string], _ map[mtime.Time]int, panesInBundle []bundlePane, schedulable bool, pendingAdjustment int) {
18951948
minTs := mtime.MaxTimestamp
@@ -2286,9 +2339,13 @@ func (ss *stageState) updateWatermarks(em *ElementManager) set[string] {
22862339
if minWatermarkHold < newOut {
22872340
newOut = minWatermarkHold
22882341
}
2289-
// If the newOut is smaller, then don't change downstream watermarks.
2342+
// If the newOut is smaller, then don't change downstream watermarks. Any
2343+
// consumer that received data still needs scheduling, since an unadvancing
2344+
// watermark is otherwise the only thing that would surface it.
22902345
if newOut <= ss.output {
2291-
return nil
2346+
refreshes := ss.consumersWithNewData
2347+
ss.consumersWithNewData = nil
2348+
return refreshes
22922349
}
22932350

22942351
// If bigger, advance the output watermark
@@ -2328,6 +2385,7 @@ func (ss *stageState) updateWatermarks(em *ElementManager) set[string] {
23282385

23292386
// Update this stage's output watermark, and then propagate that to downstream stages
23302387
refreshes := set[string]{}
2388+
ss.consumersWithNewData = nil
23312389
ss.output = newOut
23322390
for _, outputCol := range ss.outputIDs {
23332391
consumers := em.consumers[outputCol]
@@ -2435,13 +2493,20 @@ func (ss *stageState) bundleReady(em *ElementManager, emNow mtime.Time) (mtime.T
24352493
previousInputW := ss.previousInput
24362494

24372495
_, isOrdinaryStage := ss.kind.(*ordinaryStageKind)
2438-
if isOrdinaryStage && len(ss.sides) == 0 {
2496+
_, isStatefulStage := ss.kind.(*statefulStageKind)
2497+
switch {
2498+
case isOrdinaryStage && len(ss.sides) == 0:
24392499
// For ordinary stage with no side inputs, we use whether there are pending elements to determine
24402500
// whether a bundle is ready or not.
24412501
if len(ss.pending) == 0 {
24422502
return mtime.MinTimestamp, false, ptimeEventsReady, injectedReady
24432503
}
2444-
} else if inputW == upstreamW && previousInputW == inputW {
2504+
case isStatefulStage && len(ss.sides) == 0 && em.sawResidual.Load() && ss.hasBuildableDataLocked(upstreamW):
2505+
// A stateful stage processes pending data at whatever the current
2506+
// watermark is, so data alone makes it ready once something is self
2507+
// checkpointing. Side input readiness comes from the watermark, so
2508+
// stages that read one keep waiting.
2509+
case inputW == upstreamW && previousInputW == inputW:
24452510
// Otherwise, use the progression of watermark to determine the bundle readiness.
24462511
slog.Debug("bundleReady: unchanged upstream watermark",
24472512
slog.String("stage", ss.ID),

0 commit comments

Comments
 (0)