Skip to content

Abstract SnapshotWorker and durability::Local#4982

Open
Shubham8287 wants to merge 5 commits intomasterfrom
shub/persistence-abstraction
Open

Abstract SnapshotWorker and durability::Local#4982
Shubham8287 wants to merge 5 commits intomasterfrom
shub/persistence-abstraction

Conversation

@Shubham8287
Copy link
Copy Markdown
Contributor

Description of Changes

  • Introduce SnapshotRepo as the snapshot backend trait and decouple SnapshotWorker from the
    filesystem-backed implementation.

    • Dynamic dispatch-ing is not needed but done to avoid propagating generics all over the place.
    • Snapshotting is also not on the hot path, and durability also follows the same dyn pattern.
  • Make commitlog durability::Local generic over Repo, so the same durability implementation can be
    reused with in-memory and fault-injected repositories.

  • Add a few small API exposure changes under cfg(test) for DST integration.

API and ABI breaking changes

NA.

Expected complexity level and risk

2

Testing

Existing tests should be enough.

@Shubham8287 Shubham8287 force-pushed the shub/persistence-abstraction branch from 8c9bfe0 to 42e55dc Compare May 8, 2026 11:28
@Shubham8287 Shubham8287 requested review from gefjon and kim May 8, 2026 11:29
@Shubham8287 Shubham8287 changed the title Core refactor for DST. Abstract SnapshotWorker and durability::Local May 8, 2026
Comment thread crates/commitlog/src/lib.rs

#[cfg(any(feature = "test", test))]
#[tracing::instrument(level = "trace", skip_all)]
pub fn try_begin_mut_tx(&self, isolation_level: IsolationLevel, workload: Workload) -> Option<MutTx> {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems unrelated

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah, as I have put in description. They are small, trivial and under test flag.

Add a few small API exposure changes under cfg(test) for DST integration.

}

pub(crate) fn add_columns_to_table(
pub(crate) fn add_columns_to_table_mut_tx(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also unrelated?

}

#[cfg(any(feature = "test", test))]
pub fn add_columns_to_table(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here too


impl Locking {
#[cfg(any(feature = "test", test))]
pub fn try_begin_mut_tx(&self, _isolation_level: IsolationLevel, workload: Workload) -> Option<MutTxId> {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here too

Comment thread crates/commitlog/src/repo/mod.rs Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants