Simplify checkpoint pruning #3159
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
HeinrichApfelmus
changed the title
HeinrichApfelmus
force-pushed
the
HeinrichApfelmus/ADP-1043/PruneCheckpoints
branch
from
de08803
to
db36b61
Compare
… using a `CheckpointPolicy` which uses a predicate to determine whether a checkpoint should be kept.
… to the `Cardano.Wallet.DB.WalletState` module.
Use the new `extendAndPrune` function here as well.
HeinrichApfelmus
force-pushed
the
HeinrichApfelmus/ADP-1043/PruneCheckpoints
branch
from
6c1f5f5
to
09cf539
Compare
|
bors try |
tryBuild failed: |
There was a problem hiding this comment.
Hi @HeinrichApfelmus So we have CheckpointPolicy now introducing how we deal with "checkpoint denisty". What about having two checkpoint policies, sparseArithmeticPolicy and the one reflecting exactly the one in master. And having them we will compare how they perform in benchmarks. I am fond of this abstraction! At first look sparseArithemeticPolicy looks proper, rationale is sound, well know. I will give a second very soon.
😊
The idea is that Unfortunately for the entire pull request, I just noticed that light-mode will force path dependence upon us again, as we will not see every block in sequence anymore, only nondeterministic ranges of blocks. 🙈 I will have to rethink the checkpoint approach again — that is why I have moved this pull request back to draft status. Could I focus your attention to reviewing PR #3143 instead? Progressing further with light-mode will make it more clear to me how exactly I will need to adapt the new |
iohk-bors bot
added a commit
that referenced
this pull request
Jun 18, 2022
3327: Implement `CheckpointPolicy` data type r=HeinrichApfelmus a=HeinrichApfelmus ### Issue number ADP-1497 ### Overview This pull request implements an abstract data type `CheckpointPolicy` which describes a policy for keeping and discarding checkpoints based on their BlockHeight. Specifically, a boolean `keepWhereTip policy tip height` indicates whether the `policy` wants to store a checkpoint at the block height `height` , given that the blockchain is at `tip`. ### Details * The implementation of the data type is based on a function ``` nextCheckpoint policy tip height :: Maybe BlockHeight ``` which returns the next block height (`>= height`) at which a checkpoint should be made. This function is an efficient balance between the purely boolean predicate (`keepWhereTip`) and a full listing of all checkpoints (`toListAtTip`). * New policies can be created from old policies by using the semigroup operation `(<>)`, which represents the union of the checkpoint listings. This simplifies the implementation of the `sparseArithmetic` policy significantly. * The `sparseArithmetic` policy is modeled on the checkpointing policy currently implemented in `cardano-wallet`, but it is not equal to it. ### Comments * This pull request implements the data type, but does *not* use it yet, in order to keep this PR small and easy to review. The type will be used in a future pull request. * This data type is a reimplementation of the data type introduced in the stale pull request #3159. * The main reason for not merging the old PR is that a purely boolean predicate `keepWhereTip` is not suitable for use with `lightSync`. The problem is that `lightSync` does not always read individual blocks and can (potentially) skip over many BlockHeights — which would result in *no* checkpoints being created, as the predicate always returns `False`. Put differently, the checkpoints would "fall between the cracks". In contrast, an implementation based on `nextCheckpoint` can be used with `lightSync`, as it can now *specifically target* the next block height, at which a checkpoint is created. Co-authored-by: Heinrich Apfelmus <[email protected]>
iohk-bors bot
added a commit
that referenced
this pull request
Jun 19, 2022
3327: Implement `CheckpointPolicy` data type r=HeinrichApfelmus a=HeinrichApfelmus ### Issue number ADP-1497 ### Overview This pull request implements an abstract data type `CheckpointPolicy` which describes a policy for keeping and discarding checkpoints based on their BlockHeight. Specifically, a boolean `keepWhereTip policy tip height` indicates whether the `policy` wants to store a checkpoint at the block height `height` , given that the blockchain is at `tip`. ### Details * The implementation of the data type is based on a function ``` nextCheckpoint policy tip height :: Maybe BlockHeight ``` which returns the next block height (`>= height`) at which a checkpoint should be made. This function is an efficient balance between the purely boolean predicate (`keepWhereTip`) and a full listing of all checkpoints (`toListAtTip`). * New policies can be created from old policies by using the semigroup operation `(<>)`, which represents the union of the checkpoint listings. This simplifies the implementation of the `sparseArithmetic` policy significantly. * The `sparseArithmetic` policy is modeled on the checkpointing policy currently implemented in `cardano-wallet`, but it is not equal to it. ### Comments * This pull request implements the data type, but does *not* use it yet, in order to keep this PR small and easy to review. The type will be used in a future pull request. * This data type is a reimplementation of the data type introduced in the stale pull request #3159. * The main reason for not merging the old PR is that a purely boolean predicate `keepWhereTip` is not suitable for use with `lightSync`. The problem is that `lightSync` does not always read individual blocks and can (potentially) skip over many BlockHeights — which would result in *no* checkpoints being created, as the predicate always returns `False`. Put differently, the checkpoints would "fall between the cracks". In contrast, an implementation based on `nextCheckpoint` can be used with `lightSync`, as it can now *specifically target* the next block height, at which a checkpoint is created. Co-authored-by: Heinrich Apfelmus <[email protected]>
iohk-bors bot
added a commit
that referenced
this pull request
Jun 12, 2023
3988: [ADP-3031] Simplify checkpoint pruning, take 3 r=HeinrichApfelmus a=HeinrichApfelmus ### Overview In this pull request, we consolidate and simplify the creation and pruning of checkpoints. Specifically, we introduce a function `extendAndPrune` that computes a delta which * adds new checkpoints * prunes the existing checkpoints based on their block height. ### Details * The mechanism for creating checkpoints has changed. Specifically, when synchronizing the chain far away from the `tip`, at most two checkpoints are kept: genesis and the latest synchronization point. We only keep multiple checkpoints when we are within `epochStability` of the tip, as we expect rollbacks only then. * The `CheckpointPolicy` is tested in the existing module `Cardano.Wallet.Checkpoints.PolicySpec`. ### Comments * This task evolved out of ADP-1497. Previous attempts to implement this are * #3159 * #3369 ### Issue Number ADP-3031 Co-authored-by: Heinrich Apfelmus <[email protected]>
iohk-bors bot
added a commit
that referenced
this pull request
Jun 16, 2023
3988: [ADP-3031] Simplify checkpoint pruning, take 3 r=HeinrichApfelmus a=HeinrichApfelmus ### Overview In this pull request, we consolidate and simplify the creation and pruning of checkpoints. Specifically, we introduce a function `extendAndPrune` that computes a delta which * adds new checkpoints * prunes the existing checkpoints based on their block height. ### Details * The mechanism for creating checkpoints has changed. Specifically, when synchronizing the chain far away from the `tip`, at most two checkpoints are kept: genesis and the latest synchronization point. We only keep multiple checkpoints when we are within `epochStability` of the tip, as we expect rollbacks only then. * The `CheckpointPolicy` is tested in the existing module `Cardano.Wallet.Checkpoints.PolicySpec`. ### Comments * This task evolved out of ADP-1497. Previous attempts to implement this are * #3159 * #3369 ### Issue Number ADP-3031 Co-authored-by: Heinrich Apfelmus <[email protected]>
iohk-bors bot
added a commit
that referenced
this pull request
Jun 16, 2023
3988: [ADP-3031] Simplify checkpoint pruning, take 3 r=HeinrichApfelmus a=HeinrichApfelmus ### Overview In this pull request, we consolidate and simplify the creation and pruning of checkpoints. Specifically, we introduce a function `extendAndPrune` that computes a delta which * adds new checkpoints * prunes the existing checkpoints based on their block height. ### Details * The mechanism for creating checkpoints has changed. Specifically, when synchronizing the chain far away from the `tip`, at most two checkpoints are kept: genesis and the latest synchronization point. We only keep multiple checkpoints when we are within `epochStability` of the tip, as we expect rollbacks only then. * The `CheckpointPolicy` is tested in the existing module `Cardano.Wallet.Checkpoints.PolicySpec`. ### Comments * This task evolved out of ADP-1497. Previous attempts to implement this are * #3159 * #3369 ### Issue Number ADP-3031 Co-authored-by: Heinrich Apfelmus <[email protected]>
iohk-bors bot
added a commit
that referenced
this pull request
Jun 16, 2023
3988: [ADP-3031] Simplify checkpoint pruning, take 3 r=HeinrichApfelmus a=HeinrichApfelmus ### Overview In this pull request, we consolidate and simplify the creation and pruning of checkpoints. Specifically, we introduce a function `extendAndPrune` that computes a delta which * adds new checkpoints * prunes the existing checkpoints based on their block height. ### Details * The mechanism for creating checkpoints has changed. Specifically, when synchronizing the chain far away from the `tip`, at most two checkpoints are kept: genesis and the latest synchronization point. We only keep multiple checkpoints when we are within `epochStability` of the tip, as we expect rollbacks only then. * The `CheckpointPolicy` is tested in the existing module `Cardano.Wallet.Checkpoints.PolicySpec`. ### Comments * This task evolved out of ADP-1497. Previous attempts to implement this are * #3159 * #3369 ### Issue Number ADP-3031 Co-authored-by: Heinrich Apfelmus <[email protected]>
iohk-bors bot
added a commit
that referenced
this pull request
Jun 16, 2023
3988: [ADP-3031] Simplify checkpoint pruning, take 3 r=HeinrichApfelmus a=HeinrichApfelmus ### Overview In this pull request, we consolidate and simplify the creation and pruning of checkpoints. Specifically, we introduce a function `extendAndPrune` that computes a delta which * adds new checkpoints * prunes the existing checkpoints based on their block height. ### Details * The mechanism for creating checkpoints has changed. Specifically, when synchronizing the chain far away from the `tip`, at most two checkpoints are kept: genesis and the latest synchronization point. We only keep multiple checkpoints when we are within `epochStability` of the tip, as we expect rollbacks only then. * The `CheckpointPolicy` is tested in the existing module `Cardano.Wallet.Checkpoints.PolicySpec`. ### Comments * This task evolved out of ADP-1497. Previous attempts to implement this are * #3159 * #3369 ### Issue Number ADP-3031 Co-authored-by: Heinrich Apfelmus <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue number
ADP-1497
Overview
Previous work in epic ADP-1043 introduced delta encodings, DBVars, and an embedding of the wallet state and its delta encodings into a database table. It's time to integrate these tools with the wallet code. To facilitate code review, the integration proceeds in a sequence of refactorings that do not change functionality and pass all unit tests.
In this step, we consolidate and simplify the creation and pruning of checkpoints.
This step prepares the
Checkpointstype so that it can accommodate theDeltaUTxOtype: At the moment, checkpoints ofUTxOcan be pruned by simply dropping them from the Map, but in the future, checkpoints ofDeltaUTxOneed to be combined with the semigroup operation(<>). This would have been awkward to implement in the previous implementation of the checkpoint hygiene logic, which created and pruned checkpoints partially before they were written to the database, and partially after. By using a single predicatewhich determines whether to keep a checkpoint with block height
heightwhen the blockchain tip istip, the distinction between the cases "checkpoint before adding toCheckpoints" and "checkpoint after adding toCheckpoints" becomes irrelevant.Details
sparseArithmeticPolicyis modeled on the previous checkpointing policy, but it is not equal to it. Specifically, when synchronizing the chain far away from the tip, at most two checkpoints are kept: genesis and the latest synchronization point. We only keep multiple checkpoints when we are withinepochStabilityof the tip, as we expect rollbacks only then.epochStabilityof the tip may no longer be valid in the upcoming Ouroboros Genesis protocol. We may have to revisit the checkpointing logic at that time, but by then, the full incorporation of delta encodings should allow us to keep a lot of checkpoints.