Skip to content

[dbsp] Spill to storage on insert less aggressively. #5914

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ryzhyk merged 1 commit into from
Mar 25, 2026
Merged

[dbsp] Spill to storage on insert less aggressively. #5914

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions crates/dbsp/src/circuit/runtime.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1095,8 +1095,8 @@ impl Runtime {
/// Returns the minimum number of bytes in a batch inserted into a spine by a foreground worker to spill it to storage.
///
/// The output is determined by the current memory pressure level and the user-configured `min_storage_bytes` option.
/// When memory pressure is low or moderate, the output is `min_storage_bytes`, when memory pressure is high or critical,
/// the output is `0`, meaning all batches are spilled to storage.
/// When memory pressure is low the output is `usize::MAX`, when memory pressure is moderate, the output is `min_storage_bytes`,
/// when memory pressure is high or critical, the output is `0`, meaning all batches are spilled to storage.
///
/// # Returns
///
Expand All @@ -1111,12 +1111,18 @@ impl Runtime {

if inner.memory_pressure() >= MemoryPressure::High {
Some(0)
} else if inner.memory_pressure() >= MemoryPressure::Moderate {
// Moderate pressure: spill large batches to storage in the foreground; the merger will take care of the rest.
Some(
storage
.options
.min_storage_bytes
.unwrap_or(10 * 1024 * 1024),
)
} else {
Some(storage.options.min_storage_bytes.unwrap_or({
// This reduces the files stored on disk to a reasonable number.

10 * 1024 * 1024
}))
// When there is no memory pressure, we leave it to the merger to write the batches to storage
// eventually.
Some(usize::MAX)
Comment on lines 1112 to +1125
Copy link
Copy Markdown
Member

@blp blp Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might better be written as a match.

(This is petty and feel free to ignore it.)

Copy link
Copy Markdown
Contributor Author

@ryzhyk ryzhyk Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea!

}
})
}
Expand Down Expand Up @@ -2118,10 +2124,10 @@ mod tests {
query_runtime_memory_state(circuit.runtime());
assert_eq!(pressure, MemoryPressure::Low);

// Verify the spill thresholds: at low pressure, all thresholds are set to the user-configured `min_storage_bytes`
// and `min_step_storage_bytes` values.
// Verify the spill thresholds: at low pressure, merge threshold is set to the user-configured `min_storage_bytes`,
// insert threshold is set to `usize::MAX`, and step threshold is set to `usize::MAX`.
assert_eq!(min_merge, TEN_MIB);
assert_eq!(min_insert, TEN_MIB);
assert_eq!(min_insert, usize::MAX);
assert_eq!(min_step, usize::MAX);

// Verify the spine currently holds in-memory tuples before pressure rises.
Expand Down
Loading