Skip to content

Conversation

jasagredo
Copy link
Contributor

No description provided.

@jasagredo
Copy link
Contributor Author

jasagredo commented Aug 25, 2025

I plan on merging this as a PR stack:

This is a graph of the dependencies of the PRs

            ┌─────┐
            │#1653│
            └──┬──┘
               │
            ┌──▼──┐
            │#1652│
            └──┬──┘
               │
            ┌──▼──┐
            │#1603│
            └──┬──┘
               │
               │
            ┌──▼──┐
            │#1572│
            └─────┘
               │
   ┌───────┬───┴───┬───────┐
┌──▼──┐ ┌──▼──┐ ┌──▼──┐ ┌──▼──┐
│#1640│ │#1643│ │#1641│ │#1644│
└─────┘ └─────┘ └─────┘ └─────┘

@jasagredo jasagredo force-pushed the js/snapshot-manager branch from 7f77886 to 8aa430b Compare August 25, 2025 13:36
@jasagredo jasagredo force-pushed the js/snapshot-manager branch from 8aa430b to 9fca513 Compare August 25, 2025 15:18
@jasagredo jasagredo force-pushed the js/snapshot-manager branch from 9fca513 to bafbd1c Compare August 26, 2025 09:23
@jasagredo jasagredo force-pushed the js/snapshot-manager branch from bafbd1c to 6748c08 Compare August 26, 2025 11:23
@jasagredo jasagredo force-pushed the js/snapshot-manager branch from 6748c08 to bcc5b88 Compare August 27, 2025 16:04
@jasagredo jasagredo force-pushed the js/snapshot-manager branch from bcc5b88 to 26f08d1 Compare August 29, 2025 09:57
Different LedgerDB backends will manage snapshots in different ways. In
particular, before LSM trees each snapshot was fully contained in a directory in
the ledger folder of the ChainDB.

However LSM trees store part of the snapshot in the LSM database, which might be
somewhere else.

The SnapshotManagement record of functions provide a common interface for
managing the snapshots.
Copy link
Contributor

@geo2a geo2a left a comment

Choose a reason for hiding this comment

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

Seems like a sensible refactoring, I do not see anything that does not look right 👍

, deleteSnapshot :: DiskSnapshot -> m ()
, takeSnapshot ::
Maybe String ->
-- \^ The (possibly empty) suffix for the snapshot name
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: fourmolu does not like the invalid haddock here. Consider converting -- \^ to --.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I thought to keep it like this to convey the message of "I would like this to be a haddock but it can't so read it as if it was a haddock please". What do you think?

Base automatically changed from js/forkers to main September 2, 2025 23:38
@jasagredo jasagredo added this pull request to the merge queue Sep 2, 2025
Merged via the queue into main with commit 4235fd7 Sep 3, 2025
17 of 18 checks passed
@jasagredo jasagredo deleted the js/snapshot-manager branch September 3, 2025 00:46
@jasagredo jasagredo moved this to ✅ Done in Consensus Team Backlog Sep 3, 2025
@jasagredo jasagredo self-assigned this Sep 3, 2025
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