Skip to content

Commit e072b32

Browse files
authored
feat: Add list with backward checkpoint scan (delta-io#2174)
## 🥞 Stacked PR Use this [link](https://github.com/delta-io/delta-kernel-rs/pull/2174/files) to review incremental changes. - [stack/refactor-list-new](delta-io#2173) [[Files changed](https://github.com/delta-io/delta-kernel-rs/pull/2173/files)] [MERGED] - [**stack/backward-listing-new**](delta-io#2174) [[Files changed](https://github.com/delta-io/delta-kernel-rs/pull/2174/files)] --------- ## What changes are proposed in this pull request? This PR improves snapshot loading for time travel by replacing the full forward listing fallback with a backward checkpoint scan. Previously, when a _last_checkpoint hint was unavailable or pointed past the requested version, kernel fell back to listing the entire log from v0. The new strategy lists from the filesystem from the target version in 1000-version windows backwards, stopping as soon as a complete checkpoint is found. It keeps track of the files as it lists backwards and once a window with a checkpoint is found, it calls the new build_log_segment_files() function (refactored list() logic - added in previous PR in the stack) with the files. This PR introduces the following changes: - New list_with_backward_checkpoint_scan() method on LogSegmentFiles: scans backward from an end_version in 1000-version windows, stopping at the window with the first complete checkpoint - New has_complete_checkpoint_in() helper: given a slice of ParsedLogPath files, returns true if at least one complete checkpoint is present. This is used by the backward scan loop to decide whether to stop. - Updated for_snapshot_impl case dispatch - Updated build_log_segment_files(): when called with start_version: None, derives the lower bound from the most recent complete checkpoint found during the filesystem phase rather than requiring the caller to supply it explicitly. This allows list_with_backward_checkpoint_scan() to pass all scanned files in a single call and have the (checkpoint-derived) lower bound computed automatically. ## How was this change tested? For new functions introduced in log_segment_files: - Tests for list_with_backward_checkpoint_scan() behaviour - Window loop arithmetic (for listing from file system 1000 versions at a time) - has_complete_checkpoint_in() edge cases Existing and new tests that cover the new path/case in for_snapshot_impl(): - build_snapshot_without_checkpoints (second sub-test, ttv=Some(2)) - build_snapshot_with_checkpoint_greater_than_time_travel_version - build_snapshot_time_travel_no_checkpoint_falls_back_to_v0 (added with this PR) - build_snapshot_time_travel_no_hint_checkpoint_at_end_version_included (added with this PR) - test_compaction_in_version_range - test_compaction_out_of_version_range Initially opted for an [implementation similar to delta kernel java](https://sourcegraph.prod.databricks-corp.com/delta-io/delta/-/blob/kernel/kernel-api/src/main/java/io/delta/kernel/internal/checkpoints/Checkpointer.java?L182), where we have a function that finds the last complete checkpoint version, returns the version, and then we call the normal list() function from there (my [initial implementation](delta-io#2118)). This improves performance in the case where there is a nearby checkpoint to find, but in the case that there are no checkpoints, this makes the process of building a snapshot 2x slower (because we essentially list from the filesystem the same files twice). Because of this, I opted for an implementation that keeps track of the files as we list backwards. Benchmarking results: <img width="2266" height="838" alt="image" src="https://github.com/user-attachments/assets/4ed1b7d5-f640-4f42-8324-3f83b21fadbb" /> The first row of this picture shows the current state of this code, the second row is the initial implementation that causes a regression, and the third row is the current implementation.
1 parent f099eaf commit e072b32

4 files changed

Lines changed: 1095 additions & 491 deletions

File tree

kernel/src/log_segment.rs

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -298,20 +298,38 @@ impl LogSegment {
298298
.as_ref()
299299
.and_then(|hint| hint.checkpoint_schema.clone());
300300

301-
let listed_files = match (checkpoint_hint, time_travel_version) {
302-
(Some(cp), None) => {
303-
LogSegmentFiles::list_with_checkpoint_hint(&cp, storage, &log_root, log_tail, None)?
304-
}
305-
(Some(cp), Some(end_version)) if cp.version <= end_version => {
306-
LogSegmentFiles::list_with_checkpoint_hint(
307-
&cp,
308-
storage,
309-
&log_root,
310-
log_tail,
311-
Some(end_version),
312-
)?
313-
}
314-
_ => LogSegmentFiles::list(storage, &log_root, log_tail, None, time_travel_version)?,
301+
// The end_version is the time_travel_version, if present
302+
// TODO: When max catalog version is implemented, we would use that as end_version if
303+
// time_travel_version is not present
304+
let end_version = time_travel_version;
305+
306+
// Keep the hint only if it points at or before end_version, or if there is no end_version bound
307+
let usable_hint = checkpoint_hint.filter(|cp| end_version.is_none_or(|v| cp.version <= v));
308+
309+
// Cases:
310+
//
311+
// 1. usable_hint present, end_version is Some --> list_with_checkpoint_hint from hint.version TO end_version
312+
// 2. usable_hint present, end_version is None --> list_with_checkpoint_hint from hint.version unbounded
313+
// 3. no usable_hint, end_version is Some --> backward-scan for checkpoint before end_version,
314+
// list from that checkpoint TO end_version
315+
// (falls back to v0 if no checkpoint found)
316+
// 4. no usable_hint, end_version is None --> list from v0 unbounded
317+
318+
let listed_files = match (usable_hint, end_version) {
319+
// Cases 1 and 2
320+
(Some(cp), end_version) => LogSegmentFiles::list_with_checkpoint_hint(
321+
&cp,
322+
storage,
323+
&log_root,
324+
log_tail,
325+
end_version,
326+
)?,
327+
// Case 3
328+
(None, Some(end)) => LogSegmentFiles::list_with_backward_checkpoint_scan(
329+
storage, &log_root, log_tail, end,
330+
)?,
331+
// Case 4
332+
(None, None) => LogSegmentFiles::list(storage, &log_root, log_tail, None, None)?,
315333
};
316334

317335
LogSegment::try_new(

kernel/src/log_segment/tests.rs

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -829,6 +829,62 @@ async fn build_snapshot_with_start_checkpoint_and_time_travel_version() {
829829
assert_eq!(log_segment.listed.ascending_commit_files[0].version, 4);
830830
}
831831

832+
#[rstest::rstest]
833+
#[case::no_hint(None)]
834+
#[case::stale_hint(Some(LastCheckpointHint {
835+
version: 10, // stale: 10 > end_version 5, so it is discarded
836+
size: 10,
837+
parts: None,
838+
size_in_bytes: None,
839+
num_of_add_files: None,
840+
checkpoint_schema: None,
841+
checksum: None,
842+
tags: None,
843+
}))]
844+
#[tokio::test]
845+
async fn build_snapshot_time_travel_no_checkpoint_falls_back_to_v0(
846+
#[case] hint: Option<LastCheckpointHint>,
847+
) {
848+
let paths: Vec<Path> = (0..=5).map(|v| delta_path_for_version(v, "json")).collect();
849+
let (storage, log_root) = build_log_with_paths_and_checkpoint(&paths, None).await;
850+
851+
let log_segment =
852+
LogSegment::for_snapshot_impl(storage.as_ref(), log_root, vec![], hint, Some(5)).unwrap();
853+
854+
let commit_files = log_segment.listed.ascending_commit_files;
855+
let checkpoint_parts = log_segment.listed.checkpoint_parts;
856+
857+
assert_eq!(checkpoint_parts.len(), 0);
858+
let versions = commit_files.into_iter().map(|x| x.version).collect_vec();
859+
assert_eq!(versions, vec![0, 1, 2, 3, 4, 5]);
860+
}
861+
862+
#[tokio::test]
863+
async fn build_snapshot_time_travel_no_hint_checkpoint_at_end_version_included() {
864+
let (storage, log_root) = build_log_with_paths_and_checkpoint(
865+
&[
866+
delta_path_for_version(0, "json"),
867+
delta_path_for_version(1, "json"),
868+
delta_path_for_version(2, "json"),
869+
delta_path_for_version(3, "json"),
870+
delta_path_for_version(4, "json"),
871+
delta_path_for_version(5, "json"),
872+
delta_path_for_version(5, "checkpoint.parquet"),
873+
],
874+
None,
875+
)
876+
.await;
877+
878+
let log_segment =
879+
LogSegment::for_snapshot_impl(storage.as_ref(), log_root, vec![], None, Some(5)).unwrap();
880+
881+
let commit_files = log_segment.listed.ascending_commit_files;
882+
let checkpoint_parts = log_segment.listed.checkpoint_parts;
883+
assert_eq!(checkpoint_parts.len(), 1);
884+
assert_eq!(checkpoint_parts[0].version, 5);
885+
assert_eq!(commit_files.len(), 0);
886+
}
887+
832888
#[tokio::test]
833889
async fn build_table_changes_with_commit_versions() {
834890
let (storage, log_root) = build_log_with_paths_and_checkpoint(

0 commit comments

Comments
 (0)