Skip to content

Conversation

@roberto-bayardo
Copy link
Collaborator

@roberto-bayardo roberto-bayardo commented Dec 18, 2025

Persistable is a trait that is useful more generally than for K/V stores and journals. This PR pulls it up a level to replace both PersistentContiguous (persistent journal) and StorePersistent (k/v stores), allowing it to be implemented by any Storage structure.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Dec 19, 2025

Deploying monorepo with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5535035
Status: ✅  Deploy successful!
Preview URL: https://8462b60b.monorepo-eu0.pages.dev
Branch Preview URL: https://persistable-trait.monorepo-eu0.pages.dev

View logs

@roberto-bayardo roberto-bayardo force-pushed the persistable-trait branch 3 times, most recently from ae7f637 to 45c0852 Compare December 19, 2025 00:27
Base automatically changed from terminology to main December 19, 2025 01:09
@roberto-bayardo roberto-bayardo marked this pull request as ready for review December 19, 2025 01:31
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the Persistable trait to be a top-level storage construct, replacing the previous StorePersistable trait for key-value stores and PersistableContiguous for journals. The refactoring enables any storage structure to implement persistence capabilities through a unified interface.

Key changes:

  • Introduced a new top-level Persistable trait in lib.rs with methods: commit, sync, close, and destroy
  • Removed StorePersistable from the store module and PersistableContiguous from journal contiguous module
  • Updated all implementations to use the new Persistable trait with explicit Error associated type

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
storage/src/lib.rs Defines the new top-level Persistable trait with persistence operations
storage/src/store.rs Removes deprecated StorePersistable trait and updates documentation
storage/src/qmdb/store/mod.rs Implements Persistable for Store, adding sync and close methods
storage/src/qmdb/store/batch.rs Updates test trait bounds from StorePersistable to Persistable<Error = Error>
storage/src/qmdb/current/unordered/fixed.rs Implements Persistable for unordered fixed-size database, moves methods from PersistableContiguous
storage/src/qmdb/current/ordered/fixed.rs Implements Persistable for ordered fixed-size database
storage/src/qmdb/any/unordered/mod.rs Updates trait bounds and qualifies commit calls with CleanAny:: to resolve ambiguity
storage/src/qmdb/any/ordered/mod.rs Updates trait bounds and qualifies commit calls with CleanAny::
storage/src/qmdb/any/mod.rs Updates CleanAny trait to extend Persistable, removes duplicate method declarations
storage/src/qmdb/any/ext.rs Implements Persistable for AnyExt wrapper with proper method delegation
storage/src/qmdb/any/db.rs Updates trait bounds from PersistableContiguous to composition of MutableContiguous and Persistable
storage/src/ordinal/storage.rs Implements Persistable for Ordinal, reorders methods
storage/src/journal/contiguous/variable.rs Implements Persistable for variable-size journal
storage/src/journal/contiguous/tests.rs Defines local PersistableContiguous trait as composition, updates all test signatures
storage/src/journal/contiguous/mod.rs Removes PersistableContiguous trait (moved to top-level)
storage/src/journal/contiguous/fixed.rs Implements Persistable for fixed-size journal, removes redundant commit method
storage/src/journal/authenticated.rs Updates trait bounds to use Persistable instead of PersistableContiguous
storage/src/freezer/storage.rs Implements Persistable for Freezer, reorders methods

@roberto-bayardo roberto-bayardo force-pushed the persistable-trait branch 4 times, most recently from 3e90dc4 to 08b60a1 Compare December 19, 2025 02:21
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.

@commonwarexyz commonwarexyz deleted a comment from Copilot AI Dec 19, 2025
@commonwarexyz commonwarexyz deleted a comment from Copilot AI Dec 19, 2025
@roberto-bayardo roberto-bayardo force-pushed the persistable-trait branch 2 times, most recently from 9fbf418 to 9e31eb8 Compare December 19, 2025 16:21
@patrick-ogrady patrick-ogrady moved this to In Progress in Tracker Dec 19, 2025
@patrick-ogrady patrick-ogrady moved this from In Progress to Ready for Review in Tracker Dec 19, 2025
@roberto-bayardo
Copy link
Collaborator Author

Closing this, it makes more sense in the context of #2580

@github-project-automation github-project-automation bot moved this from Ready for Review to Done in Tracker Dec 20, 2025
@codecov
Copy link

codecov bot commented Dec 20, 2025

Codecov Report

❌ Patch coverage is 64.34109% with 46 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.61%. Comparing base (0a104cb) to head (5535035).
⚠️ Report is 16 commits behind head on main.

Files with missing lines Patch % Lines
storage/src/qmdb/any/ext.rs 10.00% 9 Missing ⚠️
storage/src/qmdb/current/unordered/fixed.rs 25.00% 9 Missing ⚠️
storage/src/qmdb/current/ordered/fixed.rs 12.50% 7 Missing ⚠️
storage/src/qmdb/any/ordered/mod.rs 53.84% 6 Missing ⚠️
storage/src/qmdb/store/mod.rs 0.00% 6 Missing ⚠️
storage/src/freezer/storage.rs 0.00% 5 Missing ⚠️
storage/src/ordinal/storage.rs 0.00% 4 Missing ⚠️
@@            Coverage Diff             @@
##             main    #2567      +/-   ##
==========================================
- Coverage   92.64%   92.61%   -0.04%     
==========================================
  Files         353      353              
  Lines      101323   101669     +346     
==========================================
+ Hits        93871    94159     +288     
- Misses       7452     7510      +58     
Files with missing lines Coverage Δ
storage/src/journal/authenticated.rs 91.07% <ø> (ø)
storage/src/journal/contiguous/fixed.rs 97.39% <ø> (-0.01%) ⬇️
storage/src/journal/contiguous/mod.rs 100.00% <ø> (ø)
storage/src/journal/contiguous/tests.rs 99.12% <100.00%> (ø)
storage/src/journal/contiguous/variable.rs 97.82% <ø> (ø)
storage/src/lib.rs 100.00% <100.00%> (ø)
storage/src/qmdb/any/db.rs 97.83% <ø> (ø)
storage/src/qmdb/any/mod.rs 100.00% <ø> (ø)
storage/src/qmdb/any/unordered/mod.rs 98.89% <100.00%> (-0.01%) ⬇️
storage/src/qmdb/store/batch.rs 96.92% <100.00%> (ø)
... and 8 more

... and 20 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0a104cb...5535035. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@roberto-bayardo roberto-bayardo deleted the persistable-trait branch December 20, 2025 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants