feat: add LSM timeline support with timeline layout management for old and new table versions #395

codope · 2025-07-13T06:36:26Z

Description

Implements LSM timeline reading capability for the hudi-rs core to support both v6 and v8 timeline formats, following Java implementation patterns with timeline layout abstraction. This is the foundation PR and we will follow up with future work on LSM timeline history reading, support for multiple layers of timeline compaction and manifest parsing for compacted timeline files.

Core Implementation

Timeline Loader Architecture: Added TimelineLoader to abstract different timeline storage formats.
Timeline Builder: Implemented TimelineBuilder to handle configuration resolution and loader instantiation based on table configuration.
Multi-loader Orchestration: Updated Timeline to support querying both active and archived/compacted timeline data.

Timeline {
    active_loader: TimelineLoader,
    archived_loader: Option<TimelineLoader>,
}

Layout v1: Uses LayoutOneActive (reads from .hoodie/ directory) and LayoutOneArchived (future)
Layout v2: Uses LayoutTwoActive (foundation for .hoodie/timeline/) and LayoutTwoCompacted (future)

Configuration Integration

Version Detection: Automatic timeline layout selection based on hoodie.table.version and hoodie.timeline.layout.version
Graceful Fallback: Handles invalid table configurations by defaulting to active timeline layout.
Backward Compatibility: Maintains full compatibility with existing v6 table APIs.

Related to #377

How are the changes test-covered

250/250 core tests passing
Integration tests for table reading, snapshot queries, and file slices
Backward compatibility verified with existing test data

codecov · 2025-07-13T06:46:49Z

Codecov Report

❌ Patch coverage is 41.25874% with 84 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.59%. Comparing base (72fe192) to head (4f2eded).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/core/src/timeline/loader.rs	40.00%	39 Missing ⚠️
crates/core/src/timeline/lsm_tree.rs	0.00%	14 Missing ⚠️
crates/core/src/timeline/mod.rs	45.00%	11 Missing ⚠️
crates/core/src/config/internal.rs	0.00%	9 Missing ⚠️
crates/core/src/timeline/builder.rs	66.66%	9 Missing ⚠️
crates/core/src/config/table.rs	66.66%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #395      +/-   ##
==========================================
- Coverage   89.33%   86.59%   -2.74%     
==========================================
  Files          49       52       +3     
  Lines        2747     2835      +88     
==========================================
+ Hits         2454     2455       +1     
- Misses        293      380      +87

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codope

A few note to reviewers.

crates/core/src/file_group/reader.rs

crates/core/src/timeline/lsm.rs

cpp/Cargo.toml

xushiyan · 2025-07-14T05:55:27Z

crates/core/src/timeline/active.rs

+    storage: Arc<Storage>,
+}
+
+impl ActiveTimeline {


let's take the opportunity to re-think some of the model design:

from the callers perspective, they should not care if the commits should come from active timeline history/ or archived/ because these are all timeline internal logic flow. If the caller requests require looking into compacted or archived commits, the timeline instance will need to make a call and fetch the data as needed. So at the interface level, we don't need an explicit ActiveTimeline. We can just keep it Timeline as the front-facing model.

xushiyan · 2025-07-14T06:20:24Z

crates/core/src/timeline/layout.rs

+#[derive(Debug, Clone)]
+pub enum TimelineLayoutType {
+    Active(super::active::ActiveTimeline),
+    Lsm(super::lsm::LsmTimeline),


We think of Timeline as a top level interface where callers can interact with, the layout is an internal config that resolved based on table version and timeline layout version (as you resolved in create_layout(). It'd be a bit odd to see a "Type" struct having a Timeline instance basically for choosing a Timeline to use.

How about we design it like:

Timeline remains as the top-level interface

When a Timeline is created, it resolves HudiConfigs, and create an internal model call TimelineLoader based on the configs

TimelineLoader can be an enum, it has these items:

LayoutOneActive LayoutOneArchived LayoutTwoActive LayoutTwoCompacted

Each loader will implement the logic to load its intended commit files.

When some APIs are invoked and need to load archived or compacted commits, the Timeline instance will use the needed Loaders to do IO

Timeline is responsible for collecting and stitching the Loaders' return data and returning final data to callers

We also avoid calling a LSM timeline since LSM tree is just one impl of Timeline which should be hidden away from Timeline's level

I agree, better suggestion overall! Few things if you can clarify:

We also avoid calling a LSM timeline since LSM tree is just one impl of Timeline which should be hidden away from Timeline's level

Should the LSM tree logic be a separate module that the loaders use, or should the loaders themselves be the LSM tree implementations? So, in the context of TimelineLoader::LayoutTwoActive, would this variant contain an LSMTree instance, or would it implement LSM tree logic directly?

Also, should the current Timeline::load_instants() method remain as-is, or would you prefer a different public API? For example, another API that takes a boolean to decide whether to load archived/compacted timeline. Alternatively, we could just add another API keeping load_instants() as-is.

@codope cool! keeping lsm tree specific in lsm_tree.rs is good. may not need to worry about re-using it for now - it can be used by the loaders exclusively, but somewhat positioned for future re-use

Looking closer - so load_instants() is actually a more generic helper function, it's the using of a selector in one of the init functions (new_from_storage()) that defines the behavior of timeline's instants loading. So depends on where load_instants() gets called, we just need to pass in the desired TimelineSelector

crates/core/src/timeline/lsm.rs

…d and new table versions Signed-off-by: Sagar Sumit <sagarsumit09@gmail.com>

Signed-off-by: Sagar Sumit <sagarsumit09@gmail.com>

This commit introduces the foundational architecture for supporting LSM timelines in hudi-rs, enabling compatibility with both v6 (active timeline) and v8 (LSM timeline) Hudi table formats. The key changes include: - A `TimelineLoader` enum to abstract different timeline storage formats. - A `TimelineBuilder` to handle configuration resolution and loader instantiation based on `hoodie.table.version` and `hoodie.timeline.layout.version`. - Refactored `Timeline` to use the new loader architecture while maintaining backward compatibility with existing APIs. - Graceful fallback for invalid table configurations to ensure test stability. Signed-off-by: Sagar Sumit <sagarsumit09@gmail.com>

Implements the orchestration logic within the `Timeline` struct to support querying both active and archived/compacted timeline data. Adds a new `load_instants_with_archive` method which queries the active loader and, if requested, the archived loader, then merges the results. Existing `get_completed_*` methods are updated to use this new orchestration layer while maintaining their original behavior. Signed-off-by: Sagar Sumit <sagarsumit09@gmail.com>

- Renamed lsm.rs to lsm_tree.rs to better reflect its purpose - Expanded LSMTree struct with core logic for manifest reading - Integrated LSMTree with TimelineLoader for v8 timeline support - Updated TimelineLoader to handle LayoutTwoActive using the new LSM tree logic Signed-off-by: Sagar Sumit <sagarsumit09@gmail.com>

Signed-off-by: Sagar Sumit <sagarsumit09@gmail.com>

xushiyan · 2025-08-24T19:32:31Z

crates/core/src/timeline/mod.rs

        selector: &TimelineSelector,
-        storage: &Storage,
        desc: bool,
+        include_archived: bool,


I understand the current TimelineSelector has this property too, which was my initial thought. But let's try to go with simple UX for the API: callers do not need to know if they need to load archived or not, which is timeline's internal impl - they just ask for a list of instants for a given time range (everything else that a timeline selector holds):

if no specific time given, load active instants

if a start or end time given, and falls into archived range, then load relevant archived + active instants

Timeline loader can internally decide what to load.

The only case that we might need users to set include_archived is to set it false when they might load for a time range that could be in archived but for perf reason they choose to skip archived. This is a rare advanced usage we should be good to ignore now.

Agreed. We’ll keep public API simple and make archived loading an internal concern driven by the selector range; removed the public flag for now.

crates/core/src/timeline/loader.rs

crates/core/src/timeline/lsm_tree.rs

xushiyan · 2025-08-24T19:44:07Z

crates/core/src/timeline/loader.rs

+                let mut instants = Vec::new();
+
+                for file_info in files {
+                    if file_info.name.starts_with("history/") || file_info.name.starts_with('_') {


if we can enhance the storage.list_files() to support a listing for only immediate files under the prefix, then we dont need explicit check and just grab the instant files. What is the case with having _ prefix?

it also occurs to me that we may need to enhance the listing by ignoring .crc files by default.. we'll track this separately.

Ensured sorted results; removed _ filter; ignoring .crc. Will do a shallow-listing enhancement on Storage in a follow-up.

crates/core/src/timeline/loader.rs

xushiyan · 2025-08-24T19:49:26Z

crates/core/src/timeline/lsm_tree.rs

+}
+
+pub struct LSMTree {
+    storage: Arc<Storage>,


need to take any hudi configs for LSM tree specific settings?

We'll pass configs into the LSM reader when adding archived reads to honor timeline-specific settings.

xushiyan · 2025-08-24T19:51:51Z

crates/core/src/timeline/mod.rs

+                instants.extend(archived_instants);
+
+                // Re-sort after merging
+                instants.sort_unstable();


can we be sure about loading results' order- archived instants already sorted right? then we can just concat with active ones?

Loaders now return sorted lists; we'll replace the final sort with a merge when archived reading lands.

xushiyan · 2025-08-24T19:57:13Z

crates/core/src/timeline/lsm_tree.rs

+use serde::{Deserialize, Serialize};
+use std::sync::Arc;
+
+pub const LSM_TIMELINE_DIR: &str = ".hoodie/timeline";


we need to respect "hoodie.timeline.history.path", and "hoodie.timeline.history.path" right? same for v6, we need to handle "hoodie.archivelog.folder". but we can deal with these in PR for archived timeline. the corresponding timeline loader should be responsible to take these from configs and own it

Noted; i'll wire in the configs but archived loaders will actually resolve paths from configs in the next PR.

crates/core/src/timeline/builder.rs

Signed-off-by: Sagar Sumit <sagarsumit09@gmail.com>

- Made load_archived_instants pub(crate) - Removed include_archived from public surface; added internal load_instants_internal used by Timeline methods - LayoutTwoActive now sorts results, ignores .crc, and drops the _ prefix filter - LSM manifest docs - Dropped v5 support in builder - Added TimelineSelector::has_time_filter() so Timeline knows when a time range is requested. - Updated Timeline::load_instants(...) to automatically include archived results when a time filter is present and an archived loader exists. Active is always queried first, archived appended if available. Signed-off-by: Sagar Sumit <sagarsumit09@gmail.com>

Signed-off-by: Sagar Sumit <sagarsumit09@gmail.com>

xushiyan · 2025-09-27T04:31:10Z

resuming review on this over the next 2 days

xushiyan

have some suggestions for code simplification. once addressed, we should be good to move forward

xushiyan · 2025-09-28T04:18:22Z

crates/core/src/timeline/selector.rs

+                (HudiTableConfig::BasePath, "file:///tmp/base".to_string()),
+                (HudiTableConfig::TableVersion, "6".to_string()),


Suggested change

(HudiTableConfig::BasePath, "file:///tmp/base".to_string()),

(HudiTableConfig::TableVersion, "6".to_string()),

(HudiTableConfig::BasePath, "file:///tmp/base"),

(HudiTableConfig::TableVersion, "6"),

same as above. the api supports Into<String>()

xushiyan · 2025-09-28T04:29:19Z

crates/core/src/config/table.rs

            )),
+            // Fallbacks are handled in callers; no defaults here
+            Self::ArchiveLogFolder => None,
+            Self::TimelineHistoryPath => None,


these 2 do have default values right? why not provide them through this API?

xushiyan · 2025-09-28T04:39:30Z

crates/core/src/timeline/loader.rs

+                let mut instants = Vec::new();
+
+                for file_info in files {
+                    if file_info.name.starts_with("history/") || file_info.name.ends_with(".crc") {


Suggested change

if file_info.name.starts_with("history/") || file_info.name.ends_with(".crc") {

// TODO: make `storage.list_files` api support such filtering, like ignore crc and return files only

if file_info.name.starts_with("history/") || file_info.name.ends_with(".crc") {

xushiyan · 2025-09-28T04:42:44Z

crates/core/src/timeline/loader.rs

+                    if file_info.name.starts_with("history/") || file_info.name.ends_with(".crc") {
+                        continue;
+                    }
+                    match selector.try_create_instant(file_info.name.as_str()) {


i think this API needs to handle v8 instants as the completed ones have completion ts

@codope this is the only blocker for merging. if we need to pass v8 timeline loading test cases, this parsing needs to be there.

xushiyan · 2025-09-28T04:52:40Z

crates/core/src/timeline/loader.rs

+        // Config wiring: if archived read not enabled, return behavior based on policy
+        let storage = match self {
+            TimelineLoader::LayoutOneArchived(storage)
+            | TimelineLoader::LayoutTwoArchived(storage) => storage.clone(),
+            _ => return Ok(Vec::new()), // Active loaders don't have archived parts
+        };


this storage clone does not need happen here?

xushiyan · 2025-09-28T04:55:51Z

crates/core/src/timeline/loader.rs

+        let enabled = configs
+            .get_or_default(TimelineArchivedReadEnabled)
+            .to::<bool>();
+        if !enabled {
+            let behavior = configs
+                .get_or_default(TimelineArchivedUnavailableBehavior)


looks like we just need 1 config? TimelineArchivedUnavailableBehavior default to continue will just either load archived or do nothing. even for internal config, we need to keep it simple for easy maintenance

xushiyan · 2025-09-28T04:57:13Z

crates/core/src/timeline/loader.rs

+                let archive_dir = configs
+                    .try_get(ArchiveLogFolder)
+                    .map(|v| v.to::<String>())
+                    .unwrap_or_else(|| ".hoodie/archived".to_string());


here this default value can be provided through default_value() ? the try_get() API handles it

xushiyan · 2025-09-28T04:58:18Z

crates/core/src/timeline/loader.rs

+                for file_info in files {
+                    if file_info.name.ends_with(".crc") {
+                        continue;
+                    }


i think we can skip checking .crc in this pr for simplicity. there is an open PR addresses this from storage level

codope requested a review from xushiyan as a code owner July 13, 2025 06:36

codope linked an issue Jul 13, 2025 that may be closed by this pull request

Read LSM tree timeline #377

Open

codope mentioned this pull request Jul 13, 2025

test: add basic v8 test tables #394

Merged

4 tasks

codope commented Jul 13, 2025

View reviewed changes

crates/core/src/file_group/reader.rs Show resolved Hide resolved

crates/core/src/timeline/lsm.rs Outdated Show resolved Hide resolved

xushiyan reviewed Jul 14, 2025

View reviewed changes

xushiyan added this to the release-0.5.0 milestone Jul 14, 2025

xushiyan added the feature label Jul 14, 2025

codope force-pushed the lsm-timeline-p1 branch from 98b19ae to 7b34918 Compare July 14, 2025 14:42

codope added 5 commits August 4, 2025 11:01

feat: add LSM timeline support with timeline layout management for ol…

dbb89ef

…d and new table versions Signed-off-by: Sagar Sumit <sagarsumit09@gmail.com>

undo cpp cargo change

82030a9

Signed-off-by: Sagar Sumit <sagarsumit09@gmail.com>

codope force-pushed the lsm-timeline-p1 branch from 7b34918 to 988fa21 Compare August 4, 2025 06:20

codope added 2 commits August 4, 2025 11:57

fix: license header

3fadc99

Signed-off-by: Sagar Sumit <sagarsumit09@gmail.com>

test: integrate with existing v8 test table

f7ef1af

Signed-off-by: Sagar Sumit <sagarsumit09@gmail.com>

xushiyan reviewed Aug 24, 2025

View reviewed changes

codope added 6 commits September 1, 2025 14:00

Merge remote-tracking branch 'origin/main' into lsm-timeline-p1

ad2062a

undo table mod.rs comment changes

46cf94a

Signed-off-by: Sagar Sumit <sagarsumit09@gmail.com>

new configs for archived behavior

0a3eefe

Signed-off-by: Sagar Sumit <sagarsumit09@gmail.com>

stub impl for archived v1

ab41315

Signed-off-by: Sagar Sumit <sagarsumit09@gmail.com>

chore(ci): update rust toolchain to 1.85

4f2eded

Signed-off-by: Sagar Sumit <sagarsumit09@gmail.com>

codope force-pushed the lsm-timeline-p1 branch from ea11bad to 4f2eded Compare September 8, 2025 06:08

xushiyan reviewed Sep 28, 2025

View reviewed changes

		(HudiTableConfig::BasePath, "file:///tmp/base".to_string()),
		(HudiTableConfig::TableVersion, "6".to_string()),

	if file_info.name.starts_with("history/") \|\| file_info.name.ends_with(".crc") {
	// TODO: make `storage.list_files` api support such filtering, like ignore crc and return files only
	if file_info.name.starts_with("history/") \|\| file_info.name.ends_with(".crc") {

feat: add LSM timeline support with timeline layout management for old and new table versions #395

Are you sure you want to change the base?

feat: add LSM timeline support with timeline layout management for old and new table versions #395

Uh oh!

Conversation

codope commented Jul 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Core Implementation

Configuration Integration

How are the changes test-covered

Uh oh!

codecov bot commented Jul 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

codope left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xushiyan Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xushiyan commented Sep 27, 2025

Uh oh!

xushiyan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codope commented Jul 13, 2025 •

edited

Loading

codecov bot commented Jul 13, 2025 •

edited

Loading

xushiyan Jul 14, 2025 •

edited

Loading