[Chunk Data Pack Pruner] Add block view index #6933
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -135,6 +135,9 @@ | |
| // CAUTION: | ||
| // - This function expects that `certifyingQC ` has been validated. (otherwise, the state will be corrupted) | ||
| // - The parent block must already have been ingested. | ||
| // - Attempts to extend the state with the _same block concurrently_ are not allowed. | ||
| // (will not corrupt the state, but may lead to an exception) | ||
| // Orphaned blocks are excepted. | ||
| // | ||
| // Per convention, the protocol state requires that the candidate's parent has already been ingested. | ||
| // Other than that, all valid extensions are accepted. Even if we have enough information to determine that | ||
|
|
@@ -147,6 +150,13 @@ | |
| // determine it is orphaned and drop it, attempt to ingest Y re-request the unknown parent X and repeat | ||
| // potentially very often. | ||
| // | ||
| // To ensure that all ancestors of a candidate block are correct and known to the FollowerState, some external | ||
| // ordering and queuing of incoming blocks is generally necessary (responsibility of Compliance Layer). Once a block | ||
| // is successfully ingested, repeated extension requests with this block are no-ops. This is convenient for the | ||
| // Compliance Layer after a crash, so it doesn't have to worry about which blocks have already been ingested before | ||
| // the crash. However, while running it is very easy for the Compliance Layer to avoid concurrent extension requests | ||
| // with the same block. Hence, for simplicity, the FollowerState may reject such requests with an exception. | ||
| // | ||
| // No errors are expected during normal operations. | ||
| func (m *FollowerState) ExtendCertified(ctx context.Context, candidate *flow.Block, certifyingQC *flow.QuorumCertificate) error { | ||
| span, ctx := m.tracer.StartSpanFromContext(ctx, trace.ProtoStateMutatorHeaderExtend) | ||
|
|
@@ -190,6 +200,9 @@ | |
|
|
||
| // Execute the deferred database operations as one atomic transaction and emit scheduled notifications on success. | ||
| // The `candidate` block _must be valid_ (otherwise, the state will be corrupted)! | ||
| // Therefore, they should be identified | ||
| // additional are just a no-op if they are identified as such early enough by the `checkBlockAlreadyProcessed` - though if the same extension is added concurrently, we might return a | ||
| // [storage.ErrAlreadyExists] error. | ||
|
Comment on lines
+204
to
+206
There was a problem hiding this comment.
I'm not understanding this sentence... 😅 @AlexHentschel |
||
| err = operation.RetryOnConflictTx(m.db, transaction.Update, deferredDbOps.Pending()) // No errors are expected during normal operations | ||
| if err != nil { | ||
| return fmt.Errorf("failed to persist candidate block %v and its dependencies: %w", blockID, err) | ||
|
|
@@ -201,8 +214,12 @@ | |
| // Extend extends the protocol state of a CONSENSUS PARTICIPANT. It checks | ||
| // the validity of the _entire block_ (header and full payload). | ||
| // | ||
| // CAUTION: | ||
| // - per convention, the protocol state requires that the candidate's | ||
| // parent has already been ingested. Otherwise, an exception is returned. | ||
| // - Attempts to extend the state with the _same block concurrently_ are not allowed. | ||
| // (will not corrupt the state, but may lead to an exception) | ||
| // Orphaned blocks are excepted. | ||
|
There was a problem hiding this comment. Same comment here - it isn't clear what "orphaned blocks are excepted" means. |
||
| // | ||
| // Per convention, the protocol state requires that the candidate's parent has already been ingested. | ||
| // Other than that, all valid extensions are accepted. Even if we have enough information to determine that | ||
|
|
@@ -215,6 +232,13 @@ | |
| // determine it is orphaned and drop it, attempt to ingest Y re-request the unknown parent X and repeat | ||
| // potentially very often. | ||
| // | ||
| // To ensure that all ancestors of a candidate block are correct and known to the Protocol State, some external | ||
| // ordering and queuing of incoming blocks is generally necessary (responsibility of Compliance Layer). Once a block | ||
| // is successfully ingested, repeated extension requests with this block are no-ops. This is convenient for the | ||
| // Compliance Layer after a crash, so it doesn't have to worry about which blocks have already been ingested before | ||
| // the crash. However, while running it is very easy for the Compliance Layer to avoid concurrent extension requests | ||
| // with the same block. Hence, for simplicity, the Protocol State may reject such requests with an exception. | ||
| // | ||
| // Expected errors during normal operations: | ||
| // - state.OutdatedExtensionError if the candidate block is outdated (e.g. orphaned) | ||
| // - state.InvalidExtensionError if the candidate block is invalid | ||
|
|
@@ -290,16 +314,26 @@ | |
| // If all checks pass, this method queues the following operations to persist the candidate block and | ||
| // schedules `BlockProcessable` notification to be emitted in order of increasing height: | ||
| // | ||
| // 5a. if and only if the candidate block's parent has not been certified yet: | ||
| // - store QC embedded into the candidate block | ||
| // - add the parent to the index of certified blocks (index: view → parent block's ID) | ||
| // - queue a `BlockProcessable` notification for the parent | ||
| // 5b. store candidate block and index it as a child of its parent (needed for recovery to traverse unfinalized blocks) | ||
| // 5c. if and only if we are given a `certifyingQC` | ||
| // - store this QC certifying the candidate block | ||
| // - add candidate to the index of certified blocks (index: view → candidate block's ID) | ||
| // - queue a `BlockProcessable` notification for the candidate block | ||
| // | ||
| // If `headerExtend` is called by `ParticipantState.Extend` (full consensus participant) then `certifyingQC` will be nil, | ||
| // but the block payload will be validated. If `headerExtend` is called by `FollowerState.Extend` (consensus follower), | ||
| // then `certifyingQC` must be not nil which proves payload validity. | ||
| // | ||
| // If the candidate block has already been ingested, the deferred database operations returned by this function call | ||
| // will errors with the benign sentinel [storage.ErrAlreadyExists], aborting the database transaction (without corrupting | ||
| // the protocol state). | ||
| // | ||
| // Expected errors during normal operations: | ||
| // - [state.InvalidExtensionError] if the candidate block is invalid | ||
| func (m *FollowerState) headerExtend(ctx context.Context, candidate *flow.Block, certifyingQC *flow.QuorumCertificate, deferredDbOps *transaction.DeferredDbOps) error { | ||
| span, _ := m.tracer.StartSpanFromContext(ctx, trace.ProtoStateMutatorExtendCheckHeader) | ||
| defer span.End() | ||
|
|
@@ -355,16 +389,26 @@ | |
| // STEP 5: | ||
| qc := candidate.Header.QuorumCertificate() | ||
| deferredDbOps.AddDbOp(func(tx *transaction.Tx) error { | ||
| // STEP 5a: Deciding whether the candidate's parent has already been certified or not. | ||
| // Here, we populate the [storage.QuorumCertificates] index: certified block ID → QC. Except for bootstrapping, this is the | ||
| // only place where this index is updated. Therefore, the parent is certified if and only if [storage.QuorumCertificates] | ||
| // contains an entry for `qc.BlockID`. We optimistically attempt to add a new element to the index. We receive a | ||
| // [storage.ErrAlreadyExists] sentinel if and only if step 5a has already been executed for the parent. | ||
| // CAUTION: This approach only works for database backends that support reads and writes as part of the same atomic transaction! | ||
| // For Pebble, where only batch writes with asynchronous reads are supported, the following logic has to be reworked. | ||
| err := m.qcs.StoreTx(qc)(tx) | ||
| if err != nil { | ||
| // storage.ErrAlreadyExists guarantees that 5a has already been executed for the parent. | ||
| if !errors.Is(err, storage.ErrAlreadyExists) { | ||
| return fmt.Errorf("could not store incorporated qc: %w", err) | ||
| } | ||
| } else { // no error entails that 5a has never been executed for the parent block | ||
| // add parent to index of certified blocks: | ||
| err := transaction.WithTx(operation.IndexCertifiedBlockByView(parent.View, qc.BlockID))(tx) | ||
| if err != nil { | ||
| return fmt.Errorf("could not index certified block: %w", err) | ||
| } | ||
|
|
||
| // trigger BlockProcessable for parent block above root height | ||
| if parent.Height > m.finalizedRootHeight { | ||
| tx.OnSucceed(func() { | ||
|
|
@@ -389,6 +433,13 @@ | |
| if err != nil { | ||
| return fmt.Errorf("could not store certifying qc: %w", err) | ||
| } | ||
|
|
||
| // add candidate to index of certified blocks: | ||
| err := transaction.WithTx(operation.IndexCertifiedBlockByView(candidate.Header.View, blockID))(tx) | ||
| if err != nil { | ||
| return fmt.Errorf("could not index certified block: %w", err) | ||
| } | ||
|
|
||
| tx.OnSucceed(func() { // queue a BlockProcessable event for candidate block, since it is certified | ||
| m.consumer.BlockProcessable(candidate.Header, certifyingQC) | ||
| }) | ||
|
|
@@ -735,7 +786,7 @@ | |
| // its payload, in which case the parent's seal is the same. | ||
| // * set the epoch fallback flag, if it is triggered | ||
| err = operation.RetryOnConflict(m.db.Update, func(tx *badger.Txn) error { | ||
| err = operation.IndexFinalizedBlockByHeight(header.Height, blockID)(tx) | ||
| if err != nil { | ||
| return fmt.Errorf("could not insert number mapping: %w", err) | ||
| } | ||
|
|
||
This file contains hidden or 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
This file contains hidden or 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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not clear on what this means. Excepted as in the above caution doesn't apply? Or do they trigger an exception? @AlexHentschel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it means "above caution does not apply", I don't think it is accurate. If it means "triggers an exception" I think we should say that instead.