[Util/Admin] Creating protocol snapshot from checkpoint file #5604

zhangchiqing · 2024-03-28T16:48:09Z

This PR creates an admin tool and util command to generate a protocol snapshot from latest checkpoint file.
The generated protocol snapshot file can be used along with the checkpoint file for dynamic bootstrapping an execution node.

codecov-commenter · 2024-03-28T16:54:24Z

Codecov Report

Attention: Patch coverage is 6.66667% with 70 lines in your changes are missing coverage. Please review.

Project coverage is 55.61%. Comparing base (5922cda) to head (846967e).
Report is 101 commits behind head on master.

Files	Patch %	Lines
admin/commands/storage/read_protocol_snapshot.go	0.00%	61 Missing ⚠️
cmd/execution_builder.go	0.00%	9 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5604      +/-   ##
==========================================
- Coverage   55.65%   55.61%   -0.04%     
==========================================
  Files        1037     1042       +5     
  Lines      101377   101957     +580     
==========================================
+ Hits        56424    56708     +284     
- Misses      40613    40897     +284     
- Partials     4340     4352      +12

Flag	Coverage Δ
unittests	`55.61% <6.66%> (-0.04%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

janezpodhostnik · 2024-04-01T18:09:56Z

admin/commands/storage/read_protocol_snapshot.go

+			if !ok {
+				return nil, fmt.Errorf("could not parse blocks-to-skip: %v", data)
+			}


I suppose the validator should do this validation.

peterargue · 2024-04-02T17:50:06Z

cmd/util/common/checkpoint.go

+		return nil, 0, flow.DummyStateCommitment, fmt.Errorf("could not find finalized height for sealed height %v: %w", sealedHeight, err)
+	}
+
+	return state.AtHeight(finalizedHeight), sealedHeight, commit, nil


this doesn't handle checking that the full sealing segment is included. Is that required for ENs?

flow-go/engine/access/rpc/backend/backend_network.go

Line 165 in 5429925

validSnapshot, err := b.getValidSnapshot(snapshot, 0, false)

Looks like the getValidSnapshot contains some necessary validation to the generated protocol snapshot, such as ensuring the finalized block and sealed block belong to the same epoch.

However, these checks are placed in the access backend (engine/access/rpc/backend_network.go), I think it makes more sense to be placed in a package inside protocol state, so that it can be imported by other packages, such as util / admin.

Any idea why we didn't do that in the first place? @jordanschalm

Most likely the reason is that the protocol state needs to be able to return a snapshot for every incorporated block, which includes snapshots that getValidSnapshot considers invalid for dynamic bootstrapping.

I'm happy to move the checks into the protocol package and agree that makes sense. It just needs to be differentiated from the existing snapshot API, something like func GetDynamicBootstrapSnapshot(state, referenceSnapshot).

zhangchiqing · 2024-04-03T16:45:24Z

engine/access/rpc/backend/backend_network.go

-// Expected error returns during normal operations:
-// * ErrSnapshotPhaseMismatch - snapshot does not contain a valid sealing segment
-// All other errors should be treated as exceptions.
-func (b *backendNetwork) getValidSnapshot(snapshot protocol.Snapshot, blocksVisited int, findNextValidSnapshot bool) (protocol.Snapshot, error) {


moved to snapshots.getValidSnapshot

jordanschalm

Nice work. Added mainly documentation suggestions.

Main feedback is regarding how we find the snapshot corresponding to a particular state commitment, details in this comment: #5604 (comment).

cmd/util/common/checkpoint.go

jordanschalm · 2024-04-03T18:53:42Z

cmd/util/common/checkpoint.go

+	// the finalized height that seals the given sealed height must be above the sealed height
+	// so if we iterate through each height, we should eventually find the finalized height


if we iterate through each height, we should eventually find the finalized height

This is not true in the general case. For example, suppose we have the following structure:

A <- B <- C <- D[Seal_A,Seal_B]

and suppose we are trying to find the block sealing A.

Block D seals A, however the latest sealed block as of D is not A, its B.

In this kind of scenario, we will never successfully exit this loop. In particular, the way sealing works, no snapshot exists for which Seal_A.StateCommitment is the sealed commitment.

We could solve this algorithmically by changing the scanning logic. Rather than scanning blocks and exiting when we find a block B such that Seal_B.StateCommitment is in our search set, we could exit when we find a block B such that seals.HighestInFork(B.ID()) (the latest seal as of block B) is in our search set. Then we could skip the step of translating a sealed height into a finalized height (findFinalizedHeightBySealedHeight).

Good catch and good idea!

I think it's simple here that we don't need to implement this function, but I think it could be a useful utility function to know which block finalizes the seal for a given block. We don't have an index for this type of lookup, and the search is currently missing.

The scenario could be: I submitted a transaction A, which is included in block 10, I would like to know which block seals my transaction (seals block 10).

Is there any other scenario that needed this function?

state/protocol/snapshots/dynamic_bootstrap.go

Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>

zhangchiqing · 2024-04-03T23:59:08Z

ledger/complete/wal/checkpoint_v6_reader.go

@@ -20,6 +20,7 @@ import (
 // ErrEOFNotReached for indicating end of file not reached error
 var ErrEOFNotReached = errors.New("expect to reach EOF, but actually didn't")

+// TODO: validate the header file and the sub file that contains the root hashes


will implement in a separate PR
#5623

zhangchiqing force-pushed the leo/5580-checkpoint-protocol-snapshot branch 7 times, most recently from 07164e1 to 6a0c386 Compare March 28, 2024 18:48

create protocol snapshot from checkpoint file

1e77fdc

zhangchiqing force-pushed the leo/5580-checkpoint-protocol-snapshot branch from 6a0c386 to 1e77fdc Compare March 28, 2024 20:34

zhangchiqing marked this pull request as ready for review March 28, 2024 20:34

zhangchiqing requested review from peterargue, janezpodhostnik and sideninja March 28, 2024 20:34

janezpodhostnik approved these changes Apr 1, 2024

View reviewed changes

zhangchiqing added 2 commits April 1, 2024 14:58

use validator

b87ed9f

log blocksToSkip

8c110ca

peterargue reviewed Apr 2, 2024

View reviewed changes

zhangchiqing added 2 commits April 3, 2024 09:33

refactor GetValidSnapshot

645fceb

update checkpoint.go to use GetDynamicBootstrapSnapshot

cba5888

zhangchiqing commented Apr 3, 2024

View reviewed changes

zhangchiqing requested review from jordanschalm and peterargue April 3, 2024 16:45

jordanschalm reviewed Apr 3, 2024

View reviewed changes

zhangchiqing and others added 2 commits April 3, 2024 15:19

Apply suggestions from code review

5f033da

Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>

add todo

846967e

zhangchiqing requested review from ramtinms and AlexHentschel as code owners April 3, 2024 23:58

zhangchiqing commented Apr 3, 2024

View reviewed changes

zhangchiqing requested a review from jordanschalm April 3, 2024 23:59

jordanschalm approved these changes Apr 4, 2024

View reviewed changes

fix lint

f63c06f

zhangchiqing enabled auto-merge April 4, 2024 16:01

zhangchiqing added this pull request to the merge queue Apr 4, 2024

Merged via the queue into master with commit ca8a169 Apr 4, 2024
55 checks passed

zhangchiqing deleted the leo/5580-checkpoint-protocol-snapshot branch April 4, 2024 16:36

jordanschalm mentioned this pull request Apr 10, 2024

Sync master with feature/protocol-state-kvstore #5655

Merged

zhangchiqing mentioned this pull request Apr 11, 2024

Backport v0.33 limit get block height and checkpoint protocol snapshot #5658

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Util/Admin] Creating protocol snapshot from checkpoint file #5604

[Util/Admin] Creating protocol snapshot from checkpoint file #5604

zhangchiqing commented Mar 28, 2024 •

edited

Loading

codecov-commenter commented Mar 28, 2024 •

edited

Loading

janezpodhostnik Apr 1, 2024

peterargue Apr 2, 2024

zhangchiqing Apr 3, 2024

jordanschalm Apr 3, 2024

zhangchiqing Apr 3, 2024

jordanschalm left a comment •

edited

Loading

jordanschalm Apr 3, 2024

zhangchiqing Apr 3, 2024

zhangchiqing Apr 3, 2024

zhangchiqing Apr 3, 2024 •

edited

Loading

		// the finalized height that seals the given sealed height must be above the sealed height
		// so if we iterate through each height, we should eventually find the finalized height

[Util/Admin] Creating protocol snapshot from checkpoint file #5604

[Util/Admin] Creating protocol snapshot from checkpoint file #5604

Conversation

zhangchiqing commented Mar 28, 2024 • edited Loading

codecov-commenter commented Mar 28, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jordanschalm left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhangchiqing Apr 3, 2024 • edited Loading

Choose a reason for hiding this comment

zhangchiqing commented Mar 28, 2024 •

edited

Loading

codecov-commenter commented Mar 28, 2024 •

edited

Loading

jordanschalm left a comment •

edited

Loading

zhangchiqing Apr 3, 2024 •

edited

Loading