Skip to content

Commit 8627644

Browse files
eshishkiclaude
andcommitted
[Enhancement] Add the spill-event scheduler infrastructure (default off)
Foundation for moving spillable pipeline operators off the busy-poller onto the pipeline event scheduler. Inert on its own: no operator opts in yet, so every plan keeps running exactly as before. - Spill event-notification layer: a SpillEventObservable on the spiller with sink/source subscription lists, gated on enable_event_scheduler(); complete_io fires point notifications with a query-lifetime guard (GuardedCompletion) that closes the decrement->notify use-after-free window. - block_reason primitive: the BlockReason enum, block_reason_bit(), the named<R, Mask>() compile guard, and verify_block_reason_covered() (the park-time safety net) with its fail-soft SpillMetrics counter. - Driver-core interior-block support: the INTERMEDIATE_BLOCK driver state, the supports_intermediate_wakeup() gate, the no-workable-pair release predicate, and all-operator pending_finish aggregation -- inert unless an interior operator declares a wakeup. - Shared SpillableFlatSinkMixin and the spill-process pump (channel + operator) reworked for one-shot notify. Each per-family operator PR (agg/join/sort) adds its own enable_spill_*_events kill-switch config flag on top of this base. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Evgeniy Shishkin <eshishki@gmail.com>
1 parent f8e8a78 commit 8627644

26 files changed

Lines changed: 1868 additions & 121 deletions

File tree

be/src/compute_env/spill/spill_components.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,12 @@ namespace starrocks::spill {
4545
// implements for SpillerWriter
4646
Status SpillerWriter::_decrease_running_flush_tasks() {
4747
if (_running_flush_tasks.fetch_sub(1) == 1) {
48+
// Flush-all completion: all flushes are done and the read phase may begin. The callback wrapper
49+
// (Spiller::set_flush_all_call_back) runs the client callback, acquires the input stream and
50+
// triggers the first restore. The source wakeup must fire after the whole wrapper (so a woken
51+
// reader sees the stream), and on the error path too (an error is also a wakeup). That is why the
52+
// notify is in a defer around the callback body, not a straight-line call.
53+
auto notify = DeferOp([this]() { _spiller->notify_source_observers(); });
4854
if (_flush_all_callback) {
4955
RETURN_IF_ERROR(_flush_all_callback());
5056
}

be/src/compute_env/spill/spill_metrics.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,9 @@ void SpillMetrics::install(MetricRegistry* registry) {
6868
registry->register_metric("spill_disk_bytes_used", MetricLabels().add("storage_type", "remote"),
6969
_remote_disk_bytes_used.get());
7070

71+
_parked_with_uncovered_reason_total = std::make_unique<IntCounter>(MetricUnit::OPERATIONS);
72+
registry->register_metric("spill_parked_with_uncovered_reason_total", _parked_with_uncovered_reason_total.get());
73+
7174
register_labeled(registry, "local", &_local);
7275
register_labeled(registry, "remote", &_remote);
7376
}

be/src/compute_env/spill/spill_metrics.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,18 @@ class SpillMetrics {
4848
IntGauge* local_disk_bytes_used() { return _local_disk_bytes_used.get(); }
4949
IntGauge* remote_disk_bytes_used() { return _remote_disk_bytes_used.get(); }
5050

51+
// Counter for the BlockReason check that runs when an operator parks. It increments when a wakeable
52+
// operator parks with a block_reason() that its covered_wakeups() mask does not list. In debug builds
53+
// this is a DCHECK instead. A non-zero value means the wakeup table has a gap: the operator may sleep
54+
// with nobody left to wake it. This is the counter to alert on.
55+
IntCounter* parked_with_uncovered_reason_total() { return _parked_with_uncovered_reason_total.get(); }
56+
5157
private:
5258
LabeledCounters _local;
5359
LabeledCounters _remote;
5460
std::unique_ptr<IntGauge> _local_disk_bytes_used;
5561
std::unique_ptr<IntGauge> _remote_disk_bytes_used;
62+
std::unique_ptr<IntCounter> _parked_with_uncovered_reason_total;
5663
MetricRegistry* _registry = nullptr;
5764
};
5865

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
// Copyright 2021-present StarRocks, Inc. All rights reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// https://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
#pragma once
16+
17+
#include <shared_mutex>
18+
#include <vector>
19+
20+
#include "exec/pipeline/primitives/pipeline_observer.h"
21+
#include "runtime/runtime_state.h"
22+
23+
namespace starrocks::spill {
24+
25+
// The notification layer a Spiller owns. It keeps two observer lists -- sink-side and source-side -- and
26+
// wakes them when spill IO completes. A sink-side sleeper (an operator parked OUTPUT_FULL, e.g. on a full
27+
// writer or a busy spill channel) is woken with sink_trigger(); a source-side sleeper (an operator parked
28+
// INPUT_EMPTY waiting on restore/flush IO) is woken with source_trigger(). Each list must be woken with its
29+
// own trigger; the wrong trigger does not wake the sleeper.
30+
//
31+
// notify_*_observers() runs under a shared lock, so several notifies can run at once. detach_observers()
32+
// takes the exclusive lock to clear the lists, so it cannot run while a notify is still iterating -- a
33+
// notify never touches an observer that detach is removing. The lock is taken once per IO completion (not
34+
// per chunk) and the fan-out is small (<= 3 observers per spiller), so it is cheap.
35+
//
36+
// This is deliberately not pipeline::PipeObservable: its defer_notify_sink() wakes the sink list with
37+
// source_trigger(), the wrong trigger here, so reusing it would lose every OUTPUT_FULL wakeup.
38+
class SpillEventObservable {
39+
public:
40+
// Subscribes a sink observer. A no-op in poller mode (event scheduler off): there the driver's observer
41+
// is never set, so subscribing a null pointer would crash the later notify. This gate lets the operator
42+
// call subscribe unconditionally. Prepare-phase, single-threaded.
43+
void subscribe_sink(RuntimeState* state, pipeline::PipelineObserver* o) {
44+
if (!state->enable_event_scheduler()) {
45+
return;
46+
}
47+
DCHECK(o != nullptr);
48+
std::unique_lock l(_mu);
49+
_sink_observers.push_back(o);
50+
}
51+
52+
void subscribe_source(RuntimeState* state, pipeline::PipelineObserver* o) {
53+
if (!state->enable_event_scheduler()) {
54+
return;
55+
}
56+
DCHECK(o != nullptr);
57+
std::unique_lock l(_mu);
58+
_source_observers.push_back(o);
59+
}
60+
61+
// Callable from any thread, but only after the state this predicate reads has been published.
62+
void notify_sink_observers() {
63+
std::shared_lock l(_mu);
64+
for (auto* o : _sink_observers) {
65+
o->sink_trigger();
66+
}
67+
}
68+
69+
void notify_source_observers() {
70+
std::shared_lock l(_mu);
71+
for (auto* o : _source_observers) {
72+
o->source_trigger();
73+
}
74+
}
75+
76+
// Takes the exclusive lock so it cannot race a live notify.
77+
//
78+
// On the normal spill path detach_observers is deliberately not called: a notify can fire on the
79+
// observer of an already-finished driver, and that is safe. Memory-wise the observer is alive because
80+
// every notify runs inside the IO task's query pin (the ResourceMemTrackerGuard window), and drivers
81+
// are torn down only with the QueryContext; behavior-wise a trigger on a non-blocked driver only sets
82+
// need_check_reschedule. The exclusive lock here protects the off-path callers (reset_state / explicit
83+
// teardown); the normal spill path never reaches it.
84+
void detach_observers() {
85+
std::unique_lock l(_mu);
86+
_sink_observers.clear();
87+
_source_observers.clear();
88+
}
89+
90+
// test accessor: total subscriptions across both lists.
91+
size_t observer_count() const {
92+
std::shared_lock l(_mu);
93+
return _sink_observers.size() + _source_observers.size();
94+
}
95+
96+
private:
97+
mutable std::shared_mutex _mu;
98+
std::vector<pipeline::PipelineObserver*> _sink_observers;
99+
std::vector<pipeline::PipelineObserver*> _source_observers;
100+
};
101+
102+
} // namespace starrocks::spill

be/src/compute_env/spill/spiller.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,20 @@
4343
namespace starrocks::spill {
4444
DEFINE_FAIL_POINT(spill_restore_sleep);
4545
DEFINE_FAIL_POINT(spill_restore_error);
46+
DEFINE_FAIL_POINT(spill_submit_error);
47+
DEFINE_FAIL_POINT(spill_flush_block);
48+
DEFINE_FAIL_POINT(spill_restore_block);
49+
50+
#ifdef FIU_ENABLE
51+
failpoint::OneToAnyBarrier& spill_flush_block_barrier() {
52+
static failpoint::OneToAnyBarrier barrier;
53+
return barrier;
54+
}
55+
failpoint::OneToAnyBarrier& spill_restore_block_barrier() {
56+
static failpoint::OneToAnyBarrier barrier;
57+
return barrier;
58+
}
59+
#endif
4660

4761
TQueryType::type spill_query_type(RuntimeState* state) {
4862
return state->query_options().query_type;
@@ -182,6 +196,12 @@ void Spiller::update_spilled_task_status(Status&& st) {
182196
}
183197

184198
Status Spiller::reset_state(RuntimeState* state) {
199+
// prepare() reallocates the writer/reader, but in-flight flush/restore tasks captured the previous raw
200+
// pointers. Refuse the reset while IO is in flight instead of swapping objects out from under a live
201+
// task; the caller (query-cache refill) retries once the tasks drain.
202+
if (has_running_io_tasks()) {
203+
return Status::InternalError("reset_state called while spill IO tasks are still in flight");
204+
}
185205
_spilled_append_rows = 0;
186206
_restore_read_rows = 0;
187207
RETURN_IF_ERROR(prepare(state));

be/src/compute_env/spill/spiller.h

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,12 @@
3535
#include "compute_env/spill/serde.h"
3636
#include "compute_env/spill/spill_components.h"
3737
#include "compute_env/spill/spill_metrics.h"
38+
#include "compute_env/spill/spill_observable.h"
3839
#include "compute_env/spill/spiller_factory.h"
3940
#include "compute_env/spill/task_executor.h"
4041
#include "fs/fs.h"
4142
#include "gen_cpp/InternalService_types.h"
43+
#include "gutil/macros.h"
4244
#include "runtime/mem_tracker.h"
4345
#include "runtime/runtime_state_fwd.h"
4446

@@ -169,6 +171,45 @@ class Spiller : public std::enable_shared_from_this<Spiller> {
169171

170172
const SpillProcessMetrics& metrics() { return _metrics; }
171173

174+
// Notification layer. It lives on the Spiller and survives reset_state, so the observer lists outlive
175+
// reallocation of the writer/reader.
176+
SpillEventObservable& observable() { return _observable; }
177+
178+
void notify_sink_observers() { _observable.notify_sink_observers(); }
179+
void notify_source_observers() { _observable.notify_source_observers(); }
180+
181+
// The single place that owns the completion order for spiller IO:
182+
// publish -> decrement the in-flight count -> notify.
183+
// The notify runs after both the predicates (published by publish_fn) and the count are visible, so one
184+
// notify serves every sleeper: the OUTPUT_FULL/INPUT_EMPTY predicate sleepers re-check predicates that
185+
// are already open, and a PENDING_FINISH driver gating on the count sees it already at its new value.
186+
// The caller must keep the notified observers alive across the call: a task body calls this through
187+
// defer_complete_io (the notify runs inside the task's query-lifetime guard window), and the
188+
// submit-failure compensation calls it directly on the pipeline thread, where the executing driver
189+
// itself keeps the query alive. Every completion path goes through here.
190+
template <class PublishFn>
191+
void complete_io(PublishFn&& publish_fn, bool wake_sink) {
192+
std::forward<PublishFn>(publish_fn)();
193+
decrease_in_flight_io();
194+
if (wake_sink) {
195+
notify_sink_observers();
196+
}
197+
notify_source_observers();
198+
}
199+
200+
// RAII form of complete_io for IO task bodies; see IOCompletion below. The guard must already be
201+
// scoped_begin'd; the returned object runs the completion and then guard.scoped_end() at scope exit.
202+
template <class PublishFn, class Guard>
203+
[[nodiscard]] auto defer_complete_io(PublishFn publish_fn, bool wake_sink, const Guard& guard);
204+
205+
// Counts in-flight IO across writer (flush) and reader (restore) tasks. It is in addition to the
206+
// per-writer/reader counters, which stay as lifetime gates for transient readers. It is incremented
207+
// before submit at every choke point, and decremented in the same defer as the per-task counter.
208+
void increase_in_flight_io() { _in_flight_io.fetch_add(1, std::memory_order_acq_rel); }
209+
void decrease_in_flight_io() { _in_flight_io.fetch_sub(1, std::memory_order_acq_rel); }
210+
211+
bool has_running_io_tasks() const { return _in_flight_io.load(std::memory_order_acquire) > 0; }
212+
172213
// set partitions for spiller only works when spiller has partitioned spill writer
173214
void set_partition(const std::vector<const SpillPartitionInfo*>& parititons);
174215
// init partition by `num_partitions`
@@ -303,6 +344,54 @@ class Spiller : public std::enable_shared_from_this<Spiller> {
303344
size_t _max_sorted_block_cnt = 0;
304345
std::atomic_bool _is_cancel = false;
305346
std::atomic_bool _global_spill_triggered{false};
347+
348+
SpillEventObservable _observable;
349+
// Count of in-flight flush/restore IO tasks. It survives reset_state (it lives on the Spiller, not on
350+
// the writer/reader), so it can gate teardown and re-prepare against tasks that captured raw
351+
// writer/reader pointers.
352+
std::atomic<int64_t> _in_flight_io{0};
353+
};
354+
355+
// RAII completion for an IO task body, returned by Spiller::defer_complete_io. At scope exit it runs the
356+
// completion (complete_io: publish -> decrement -> notify) and then the guard's scoped_end, in that order.
357+
// The order matters for correctness, not for style: scoped_end releases the query-lifetime pin the guard
358+
// took in scoped_begin, and that pin is what keeps the notified observers alive -- if scoped_end ran first,
359+
// the notify could dereference freed observers. Do not reorder. cancel_for_yield() drops the completion AND
360+
// marks the yield context unfinished in one call (on a yield-resubmit the task lives on, keeps its
361+
// in-flight increment, and re-enters scoped_begin on its next run); scoped_end still runs to balance this
362+
// invocation's begin. The two cancels are coupled on purpose: cancelling the completion without cancelling
363+
// the yield defer would let the executor drop the task with the in-flight count still held.
364+
template <class PublishFn, class Guard>
365+
class IOCompletion {
366+
public:
367+
IOCompletion(Spiller* spiller, PublishFn publish_fn, bool wake_sink, const Guard& guard)
368+
: _spiller(spiller), _publish_fn(std::move(publish_fn)), _wake_sink(wake_sink), _guard(guard) {}
369+
~IOCompletion() noexcept {
370+
if (!_cancelled) {
371+
_spiller->complete_io(_publish_fn, _wake_sink);
372+
}
373+
_guard.scoped_end();
374+
}
375+
template <class YieldDefer>
376+
void cancel_for_yield(YieldDefer& yield_defer) {
377+
_cancelled = true;
378+
yield_defer.cancel();
379+
}
380+
DISALLOW_COPY_AND_MOVE(IOCompletion);
381+
382+
private:
383+
Spiller* _spiller;
384+
PublishFn _publish_fn;
385+
const bool _wake_sink;
386+
const Guard& _guard;
387+
bool _cancelled = false;
306388
};
307389

390+
template <class PublishFn, class Guard>
391+
[[nodiscard]] auto Spiller::defer_complete_io(PublishFn publish_fn, bool wake_sink, const Guard& guard) {
392+
// Returned as a prvalue: C++17 guaranteed elision constructs it in the caller's variable, so the
393+
// deleted copy/move constructors are never used.
394+
return IOCompletion<PublishFn, Guard>(this, std::move(publish_fn), wake_sink, guard);
395+
}
396+
308397
} // namespace starrocks::spill

0 commit comments

Comments
 (0)