Chain ranged export #9192

frrist · 2022-08-20T01:42:36Z

Related Issues

Proposed Changes

Additional Info

Checklist

Before you mark the PR ready for review, please make sure that:

All commits have a clear commit message.
The PR title is in the form of of <PR type>: <area>: <change being made>
- example: fix: mempool: Introduce a cache for valid signatures
- PR type: fix, feat, INTERFACE BREAKING CHANGE, CONSENSUS BREAKING, build, chore, ci, docs,perf, refactor, revert, style, test
- area: api, chain, state, vm, data transfer, market, mempool, message, block production, multisig, networking, paychan, proving, sealing, wallet, deps
This PR has tests for new functionality or change in behaviour
If new user-facing features are introduced, clear usage guidelines and / or documentation updates should be included in https://lotus.filecoin.io or Discussion Tutorials.
CI is green

frrist · 2022-10-19T17:11:54Z

chain/store/snapshot.go

+	Dag
+)
+
+func (s *walkScheduler) work(ctx context.Context, todo *walkTask, results chan *walkResult) error {


This method needs review

frrist · 2022-10-19T17:12:08Z

node/impl/full/chain.go

@@ -587,6 +590,80 @@ func (m *ChainModule) ChainGetMessage(ctx context.Context, mc cid.Cid) (*types.M
 	return cm.VMMessage(), nil
 }

+func (a ChainAPI) ChainExportRangeInternal(ctx context.Context, head, tail types.TipSetKey, cfg *api.ChainExportConfig) error {


this file needs review

hsanjuan · 2022-10-26T13:57:26Z

chain/store/snapshot.go

@@ -24,6 +28,35 @@ func (cs *ChainStore) UnionStore() bstore.Blockstore {
 	return bstore.Union(cs.stateBlockstore, cs.chainBlockstore)
 }

+func (cs *ChainStore) ExportRange(ctx context.Context, head, tail *types.TipSet, messages, receipts, stateroots bool, workers int64, cacheSize int, w io.Writer) error {


ensure io.Writer is buffered.

hsanjuan · 2022-10-26T14:06:23Z

chain/store/snapshot.go

+	cfg  *walkSchedulerConfig
+}
+
+func (s *walkScheduler) enqueueIfNew(task *walkTask) {


no need to pass pointer?

hsanjuan · 2022-10-26T14:32:28Z

chain/store/snapshot.go

+			close(s.in)
+		}()
+		for {
+			if n := len(s.stack) - 1; n >= 0 {


comment: this does depth traversal (using the stack). The two channels (in out), we assume, add backpressure so that stack couldn't grow indifinetely if the disk is not fast enough (not completely sure).

Doing breadth-first search might allow not using the stack, and directly relying on the in channel as FIFO queue.

hsanjuan · 2022-10-26T14:33:07Z

chain/store/snapshot.go

+	Dag
+)
+
+func (s *walkScheduler) work(ctx context.Context, todo *walkTask, results chan *walkResult) error {


try avoid pointers for walkTas and walkResult.

hsanjuan · 2022-10-26T15:14:26Z

chain/store/snapshot.go

+	}
+
+	pw, ctx := newWalkScheduler(ctx, store, cfg, tasks...)
+	results := make(chan *walkResult)


make me a good channel buffer on this shit forrest

hsanjuan · 2022-10-26T14:44:06Z

chain/store/snapshot.go

+	results <- &walkResult{c: todo.c}
+
+	// extract relevant dags to walk from the block
+	if todo.taskType == 0 {


write: Block

hsanjuan · 2022-10-26T14:56:17Z

chain/store/snapshot.go

+			log.Infow("block export", "height", b.Height)
+		}
+		if b.Height == 0 {
+			log.Infow("GENESIS")


Perhaps exporting including height 0 crashes... prob not related to this part of the code.

hsanjuan · 2022-10-26T14:59:36Z

chain/store/snapshot.go

+	pw.startScheduler(ctx)
+	pw.startWorkers(ctx, results)
+
+	done := make(chan struct{})


hsanjuan · 2022-10-26T15:03:20Z

chain/store/snapshot.go

+
+	done := make(chan struct{})
+	var cbErr error
+	go func() {


potential goroutine leak if things error somewhere, as close(results) is down below.

results is now always closed: https://github.com/filecoin-project/lotus/pull/9192/files#diff-740d4bd1f10c65674bc1d0a54f1fc4192d8008b0c08179a300b42116ea0454a9R408

hsanjuan · 2022-10-26T15:09:49Z

chain/store/snapshot.go

+		defer func() {
+			log.Infow("walkScheduler shutting down")
+			close(s.out)
+			// Because the workers may have exited early (due to the context being canceled).
+			for range s.out {
+				s.taskWg.Done()
+			}
+			// Because the workers may have enqueued additional tasks.
+			for range s.in {
+				s.taskWg.Done()
+			}
+			// now, the waitgroup should be at 0, and the goroutine that was _waiting_ on it should have exited.
+			log.Infow("walkScheduler stopped")


need to re-approach synchronization here. Seems last info message never happens. Things hang on ctrl-c.

hsanjuan · 2022-10-26T16:09:38Z

At the end of the export the following is printer:

      INFO    chainstore      store/snapshot.go:283   block export    {"height": "897000"}
2022-10-26T16:07:50.734Z        INFO    chainstore      store/snapshot.go:283   block export    {"height": "897000"}
2022-10-26T16:07:50.735Z        INFO    chainstore      store/snapshot.go:283   block export    {"height": "897000"}
2022-10-26T16:07:50.735Z        INFO    chainstore      store/snapshot.go:283   block export    {"height": "897000"}
2022-10-26T16:07:50.735Z        INFO    chainstore      store/snapshot.go:283   block export    {"height": "897000"}
....

where height goes down to 0. That is very confusing since we are only doing a range export. Are we missing a check to finish on the starting height?

hsanjuan · 2022-10-27T10:32:38Z

chain/store/snapshot.go

 		}
-		if b.Height%1000 == 0 {
+		if b.Height%1_000 == 0 {


hmmm....what does this mean?

emit a log message every 1000 blocks
the underscore is serving as a comma for readability https://go.dev/doc/go1.13#language

frrist · 2022-12-01T17:32:28Z

One outstanding issue with this PR is that it sometimes cannot be stopped with Ctrl+C

- will hopfully make things faster by not streaming the export over the json rpc api

jennijuju · 2022-12-08T20:50:58Z

We need this before nv18 for stable snapshot service

magik6k · 2023-01-23T10:24:52Z

api/api_full.go

@@ -171,6 +171,10 @@ type FullNode interface {
 	// If oldmsgskip is set, messages from before the requested roots are also not included.
 	ChainExport(ctx context.Context, nroots abi.ChainEpoch, oldmsgskip bool, tsk types.TipSetKey) (<-chan []byte, error) //perm:read

+	ChainExportRange(ctx context.Context, head, tail types.TipSetKey, cfg *ChainExportConfig) (<-chan []byte, error) //perm:read


filecoin-project/go-jsonrpc#88 adds support for calling methods on the client.

A callback accepting bytes on the client would be better as that lets you have backpressure, and is arguably more robust in general.

(see https://github.com/filecoin-project/lotus/pull/10027/files for an example of how it's integrated in lotus)

hsanjuan · 2023-01-30T16:26:29Z

I'm continuing this on #10145 . I'm leaving this branch untouched since we are actually using it in production now (well, a different rebase on v0.19.0).

frrist requested a review from travisperson September 8, 2022 15:09

frrist self-assigned this Sep 23, 2022

frrist commented Oct 19, 2022

View reviewed changes

hsanjuan suggested changes Oct 26, 2022

View reviewed changes

hsanjuan reviewed Oct 27, 2022

View reviewed changes

travisperson marked this pull request as ready for review November 29, 2022 18:03

travisperson requested a review from a team as a code owner November 29, 2022 18:03

frrist and others added 9 commits December 1, 2022 22:00

works

46290f0

works against mainnet and calibnet

bef5623

feat: add internal export api method

74663b2

- will hopfully make things faster by not streaming the export over the json rpc api

polish: better file nameing

cd42438

fix: potential race in marking cids as seen

935a404

chore: improve logging

b04ecd9

feat: front export with cache

14bac9d

fix: give hector a good channel buffer on this shit

1d2d821

docsgen

0193c97

jennijuju force-pushed the frrist/chain-export-range branch from ad6ef57 to 0193c97 Compare December 2, 2022 03:03

jennijuju added the P2 P2: Should be resolved label Dec 8, 2022

jennijuju assigned travisperson and unassigned frrist Dec 8, 2022

jennijuju unassigned travisperson Jan 2, 2023

magik6k reviewed Jan 23, 2023

View reviewed changes

hsanjuan mentioned this pull request Jan 30, 2023

perf: chain: export-range #10145

Merged

7 tasks

hsanjuan closed this Jan 30, 2023

rjan90 deleted the frrist/chain-export-range branch February 3, 2025 08:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chain ranged export #9192

Chain ranged export #9192

frrist commented Aug 20, 2022

frrist Oct 19, 2022

frrist Oct 19, 2022

hsanjuan Oct 26, 2022

frrist Oct 27, 2022

hsanjuan Oct 26, 2022

hsanjuan Oct 26, 2022

hsanjuan Oct 26, 2022

hsanjuan Oct 26, 2022

hsanjuan Oct 26, 2022

hsanjuan Oct 26, 2022

hsanjuan Oct 26, 2022

hsanjuan Oct 26, 2022

frrist Oct 27, 2022

hsanjuan Oct 26, 2022

hsanjuan commented Oct 26, 2022

hsanjuan Oct 27, 2022

frrist Oct 27, 2022

frrist commented Dec 1, 2022

jennijuju commented Dec 8, 2022

magik6k Jan 23, 2023

magik6k Jan 23, 2023

hsanjuan commented Jan 30, 2023

Chain ranged export #9192

Chain ranged export #9192

Conversation

frrist commented Aug 20, 2022

Related Issues

Proposed Changes

Additional Info

Checklist

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

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

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

hsanjuan commented Oct 26, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frrist commented Dec 1, 2022

jennijuju commented Dec 8, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hsanjuan commented Jan 30, 2023