Skip to content

[HOTFIX] applying PR #5321 to release/2025-08-15 #5325

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.

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

Open
wants to merge 1 commit into
base: release/2025-08-15
Choose a base branch
from
Open
Changes from all commits
Commits
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
54 changes: 46 additions & 8 deletions rust/garbage_collector/src/garbage_collector_orchestrator_v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,11 @@ impl GarbageCollectorOrchestrator {
);

if output.file_paths.is_empty() {
// We only allow empty file paths if the version is 0 and all ancestors are also at v0. Otherwise, compaction should have flushed new file paths. This check is defensive and should never fail.
// We only allow empty file paths until the first version with file paths. After that version, all subsequent versions must have file paths. This check is defensive and should never fail. What we expect:
// v0: no data has been compacted, so there are no file paths.
// v1-N: a compaction has occurred, but it was a no-op (e.g. all log entries were deletions of non-existent IDs), so there are no file paths.
// vN+1..: a compaction has occurred with at least 1 valid log entry after materializing the logs, so there must be file paths.

let graph = self
.graph
.as_ref()
Expand Down Expand Up @@ -675,15 +679,15 @@ impl GarbageCollectorOrchestrator {
for res in extracted_paths.into_iter() {
have_paths.push(res?);
}
let have_hole_in_paths = have_paths
let has_no_paths_at_version = have_paths
.into_iter()
.skip_while(|&x| x.0 == 0 && !x.1)
.any(|x| !x.1);
.skip_while(|&(_version, has_paths)| !has_paths)
.find(|(_version, has_paths)| !has_paths);

if have_hole_in_paths {
if let Some((version, _)) = has_no_paths_at_version {
return Err(GarbageCollectorError::InvariantViolation(format!(
"Version {} of collection {} has no file paths, but has non-v0 ancestors. This should never happen.",
output.version, output.collection_id
"Version {} of collection {} has no file paths, but has ancestors with file paths. This should never happen.",
version, output.collection_id
)));
}
}
Expand Down Expand Up @@ -1216,10 +1220,44 @@ mod tests {
// Create v1 with no file paths
sysdb
.flush_compaction(
tenant,
tenant.clone(),
root_collection_id,
0,
0,
Arc::new([SegmentFlushInfo {
segment_id,
file_paths: HashMap::new(),
}]),
0,
0,
)
.await
.unwrap();

// Create v2 with file paths
sysdb
.flush_compaction(
tenant.clone(),
root_collection_id,
0,
1,
Arc::new([SegmentFlushInfo {
segment_id,
file_paths: HashMap::from([("foo".to_string(), vec!["bar".to_string()])]),
}]),
0,
0,
)
.await
.unwrap();

// Create v3 with no file paths
sysdb
.flush_compaction(
tenant,
root_collection_id,
0,
2,
Arc::new([SegmentFlushInfo {
segment_id,
file_paths: HashMap::new(),
Expand Down
Loading