Skip to content

Commit d544e3b

Browse files
committed
fix(join): fall back to in-memory sub-partitions on grace repartition
repartition_side previously treated any failure to create a sub-partition SpillPartitionWriter as a hard Storage error, even though src reaching this function already proves io_uring was available on the prior write path. Under heavy concurrent join load, io_uring / RLIMIT_MEMLOCK exhaustion can make writer creation start failing transiently at a deeper repartition level. Detect the first failed writer creation and fall back to buffering that side's rows into in-memory sub-partitions instead of erroring out, mirroring the existing graceful degradation on the push path (grace_spill::push_row). No row is ever dropped on either path; only the on-disk memory bound is lost when io_uring is unavailable.
1 parent 6256bec commit d544e3b

2 files changed

Lines changed: 90 additions & 47 deletions

File tree

nodedb/src/data/executor/handlers/join/grace_repartition.rs

Lines changed: 80 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -168,29 +168,35 @@ impl PartitionSource {
168168
}
169169

170170
/// Stream `src`'s rows frame-by-frame and re-hash each into one of `sub_p`
171-
/// new spill files under `sub_dir`, keyed by `partition_hash_seeded(row, keys,
172-
/// seed) % sub_p`. Returns the per-sub-partition spill paths (length `sub_p`).
171+
/// sub-partitions, keyed by `partition_hash_seeded(row, keys, seed) % sub_p`.
172+
/// Returns the per-sub-partition [`PartitionSource`]s (length `sub_p`).
173173
///
174-
/// Each output is a [`SpillPartitionWriter`]; the row is appended straight to
175-
/// disk so no sub-partition is ever fully resident. The reader holds at most one
176-
/// row in RAM at a time.
174+
/// Normally each output is a [`PartitionSource::Spilled`] file: rows are appended
175+
/// straight to disk via a [`SpillPartitionWriter`], so no sub-partition is ever
176+
/// fully resident (the reader holds at most one row in RAM at a time).
177177
///
178178
/// # io_uring fallback
179179
///
180-
/// `src` reaches this function ONLY when it is a [`PartitionSource::Spilled`]
181-
/// file, which means a `SpillPartitionWriter` was successfully created on the
182-
/// write path — so io_uring (or its `std::fs` fallback) is available and the
183-
/// sub-partition writers below can likewise be created. If a writer cannot be
184-
/// created we treat it as a hard error (`Storage`) rather than silently dropping
185-
/// rows, because dropping rows would corrupt the join result.
180+
/// Creating a sub-partition writer requires an io_uring instance. Even though
181+
/// `src` reaching this function proves io_uring WAS available on the earlier
182+
/// write path, it can become transiently unavailable here — under heavy parallel
183+
/// load many concurrent joins can saturate the per-process io_uring / locked-
184+
/// memory (`RLIMIT_MEMLOCK`) limits, so `SpillPartitionWriter::create` starts
185+
/// returning `None`. When ANY sub-partition writer cannot be created we fall back
186+
/// to routing every row of this side into in-memory sub-partition buffers and
187+
/// return [`PartitionSource::InMemory`]s instead. This mirrors the identical
188+
/// graceful degradation on the push path (`grace_spill::push_row`): dropping the
189+
/// on-disk memory bound for this level is an acceptable platform-capability gap
190+
/// ("correctness preserved, only the memory bound is lost"), whereas failing the
191+
/// whole join — or dropping rows — is not. A row is NEVER dropped on either arm.
186192
pub(super) fn repartition_side<S: AsRef<str>>(
187193
src: PartitionSource,
188194
keys: &[S],
189195
seed: u64,
190196
sub_p: usize,
191197
sub_dir: &Path,
192198
side_tag: &str,
193-
) -> crate::Result<Vec<PathBuf>> {
199+
) -> crate::Result<Vec<PartitionSource>> {
194200
std::fs::create_dir_all(sub_dir).map_err(|e| crate::Error::Storage {
195201
engine: "grace-repartition".into(),
196202
detail: format!(
@@ -199,52 +205,87 @@ pub(super) fn repartition_side<S: AsRef<str>>(
199205
),
200206
})?;
201207

202-
// Create one writer per sub-partition up front. `SpillPartitionWriter` is not
203-
// `Clone`, so build the Vec with an iterator.
208+
// Try to create one spill writer per sub-partition up front. `create`
209+
// returns `None` when io_uring is unavailable/exhausted; on the FIRST such
210+
// failure abandon the spill path entirely and re-partition this side into
211+
// RAM instead (see the io_uring-fallback note above). `SpillPartitionWriter`
212+
// is not `Clone`, so build the Vec with a loop.
204213
let mut writers: Vec<SpillPartitionWriter> = Vec::with_capacity(sub_p);
214+
let mut spill_ok = true;
205215
for sp in 0..sub_p {
206216
let path = sub_dir.join(format!("sp{sp}.{side_tag}.spill"));
207-
let w = SpillPartitionWriter::create(&path).ok_or_else(|| crate::Error::Storage {
208-
engine: "grace-repartition".into(),
209-
detail: format!(
210-
"failed to create sub-partition spill writer {}",
211-
path.display()
212-
),
217+
match SpillPartitionWriter::create(&path) {
218+
Some(w) => writers.push(w),
219+
None => {
220+
spill_ok = false;
221+
break;
222+
}
223+
}
224+
}
225+
226+
if spill_ok {
227+
// Spill path: route each row to its sub-partition writer, then finish
228+
// each writer into a Spilled source.
229+
route_rows(src, keys, seed, sub_p, |sp, value| {
230+
writers
231+
.get_mut(sp)
232+
.ok_or_else(|| sub_index_error(sp, sub_p))?
233+
.append_row(value)
213234
})?;
214-
writers.push(w);
235+
let mut out = Vec::with_capacity(sub_p);
236+
for w in writers {
237+
out.push(PartitionSource::Spilled(w.finish()?));
238+
}
239+
Ok(out)
240+
} else {
241+
// In-memory fallback: discard any partially-created writers (their empty
242+
// files live under `sub_dir` and are cleaned by the caller's
243+
// `remove_dir_all`) and buffer rows in RAM instead.
244+
drop(writers);
245+
let mut buffers: Vec<Vec<(String, Vec<u8>)>> = vec![Vec::new(); sub_p];
246+
route_rows(src, keys, seed, sub_p, |sp, value| {
247+
buffers
248+
.get_mut(sp)
249+
.ok_or_else(|| sub_index_error(sp, sub_p))?
250+
.push((String::new(), value.to_vec()));
251+
Ok(())
252+
})?;
253+
Ok(buffers.into_iter().map(PartitionSource::InMemory).collect())
215254
}
255+
}
216256

217-
// Stream the source and route each row by the SEEDED hash.
257+
/// Stream every row of `src` (in-memory rows by move, or a spilled file frame-by-
258+
/// frame — one row resident at a time), compute each row's sub-partition index
259+
/// via `partition_hash_seeded(row, keys, seed) % sub_p`, and hand `(sp, value)`
260+
/// to `sink`. Consumes `src`. The `sp` index is always `< sub_p` (modulo result),
261+
/// so `sink` can index safely; it returns `Err` only on a genuine I/O failure.
262+
fn route_rows<S, F>(
263+
src: PartitionSource,
264+
keys: &[S],
265+
seed: u64,
266+
sub_p: usize,
267+
mut sink: F,
268+
) -> crate::Result<()>
269+
where
270+
S: AsRef<str>,
271+
F: FnMut(usize, &[u8]) -> crate::Result<()>,
272+
{
218273
match src {
219274
PartitionSource::InMemory(rows) => {
220275
for (_, value) in rows {
221276
let sp = (partition_hash_seeded(&value, keys, seed) % sub_p as u64) as usize;
222-
// `sp < sub_p` by construction of the modulo; index is safe but
223-
// use `get_mut` to avoid any panic path in lib code.
224-
let w = writers
225-
.get_mut(sp)
226-
.ok_or_else(|| sub_index_error(sp, sub_p))?;
227-
w.append_row(&value)?;
277+
sink(sp, &value)?;
228278
}
229279
}
230280
PartitionSource::Spilled(path) => {
231281
let mut reader = FrameStreamReader::open(&path)?;
232282
while let Some(value) = reader.next_row()? {
233283
let sp = (partition_hash_seeded(&value, keys, seed) % sub_p as u64) as usize;
234-
let w = writers
235-
.get_mut(sp)
236-
.ok_or_else(|| sub_index_error(sp, sub_p))?;
237-
w.append_row(&value)?;
284+
sink(sp, &value)?;
238285
}
239286
}
240287
}
241-
242-
// Finish each writer, collecting the sub-partition spill paths in order.
243-
let mut paths = Vec::with_capacity(sub_p);
244-
for w in writers {
245-
paths.push(w.finish()?);
246-
}
247-
Ok(paths)
288+
Ok(())
248289
}
249290

250291
/// Construct the (unreachable-in-practice) sub-partition index error without

nodedb/src/data/executor/handlers/join/grace_spill.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -364,15 +364,15 @@ impl PartitionedSpiller {
364364
let sub_dir = spill_dir.join(format!("rp-{next_repartition_id}-d{}", item.depth + 1));
365365
next_repartition_id += 1;
366366

367-
let build_sub_paths = repartition_side(
367+
let build_subs = repartition_side(
368368
item.build,
369369
&build_key_refs,
370370
new_seed,
371371
SUB_P,
372372
&sub_dir,
373373
"build",
374374
)?;
375-
let probe_sub_paths = repartition_side(
375+
let probe_subs = repartition_side(
376376
item.probe,
377377
&probe_key_refs,
378378
new_seed,
@@ -382,13 +382,15 @@ impl PartitionedSpiller {
382382
)?;
383383

384384
// Enqueue SUB_P new items at depth+1. `repartition_side` always
385-
// returns exactly SUB_P paths each, positionally aligned by sub-
386-
// partition index, so the zip pairs build/probe sub-partitions that
387-
// share a seed bucket.
388-
for (bp, pp) in build_sub_paths.into_iter().zip(probe_sub_paths) {
385+
// returns exactly SUB_P sub-partition sources each, positionally
386+
// aligned by sub-partition index, so the zip pairs build/probe sub-
387+
// partitions that share a seed bucket. Each source is Spilled on the
388+
// normal path or InMemory when io_uring was unavailable at this level
389+
// (graceful fallback — see `repartition_side`).
390+
for (bp, pp) in build_subs.into_iter().zip(probe_subs) {
389391
queue.push(WorkItem {
390-
build: PartitionSource::Spilled(bp),
391-
probe: PartitionSource::Spilled(pp),
392+
build: bp,
393+
probe: pp,
392394
depth: item.depth + 1,
393395
});
394396
}

0 commit comments

Comments
 (0)