feat: PoSt workers #7971

magik6k · 2022-01-18T18:56:12Z

Related Issues

Based on #7600

Proposed Changes

PoSt workers:

Winning and Window PoSt
Multiple PoSt workers can be connected to a lotus-miner process
- In multi-partition PoSt, it should get run on multiple workers in parallel, up to the number of partitions
PoSt workers can read challenges from remote storage
- This meant that lotus-miner (nor PoSt workers) no longer need filesystem access to sectors;
Handling of corrupted sectors should be much more robust
- If we fail to read challenges for a sector, we'll skip it and retry without trying to create a bad post snark

Additional Info

This PR introduces a number of new features:

Window and Winning PoSt Workers
- PoSt workers are specialized instances of lotus-worker - one lotus-worker instance can only be one of:
  - WindowPoSt worker
  - WinningPoSt worker
  - Worker for other sealing tasks
- When a [winning/window]PoSt worker connects to lotus-miner, the miner will delegate all [winning/window]PoSt tasks to those workers - no PoSt tasks will be executed locally on the miner instance
WindowPoSt partitions are computed in parallel
- This should make it much easier to respond to WindowPoSt on time with multiple partitions per deadline
Responding to PoSt challenges from remote storage
- PoSt workers don't need local access to sectors - workers computing PoSt will ask other workers to read challenges from their storage if PoSt workers don't have access to sector storage.

Usage

(update lotus-miner and all workers (at the very least all workers with access to storage paths; post workers can read challanges directly from other workers, and if old workers are called, PoSt will fail))
Have a lotus-miner running
Start lotus-worker run [--winningpost/--windowpost] (use the same envvars like for any other worker)
See the worker registered in lotus-miner sealing workers
In miner info, see the PoSt worker, e.g. Workers: Seal(1) WdPoSt(0) WinPoSt(1)

Followups

Redundant PoSt - compute the same PoSt on multiple workers, submit first valid result
Ability to entirely disable PoSt on the main lotus-miner instance - right now this happens after PoSt workers get connected

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, misc,perf, refactor, revert, style, test
- area: api, chain, state, vm, data transfer, market, mempool, message, block production, multisig, networking, paychan, proving, sealing, wallet
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

…/lotus into feat/decoupling-post-worker

codecov · 2022-01-20T14:11:28Z

Codecov Report

Merging #7971 (7a009ab) into master (d437f19) will increase coverage by 0.31%.
The diff coverage is 70.53%.

@@            Coverage Diff             @@
##           master    #7971      +/-   ##
==========================================
+ Coverage   40.23%   40.55%   +0.31%     
==========================================
  Files         684      687       +3     
  Lines       74655    75230     +575     
==========================================
+ Hits        30040    30509     +469     
- Misses      39372    39420      +48     
- Partials     5243     5301      +58

Impacted Files	Coverage Δ
api/api_storage.go	`0.00% <ø> (ø)`
cmd/lotus-miner/init.go	`0.00% <0.00%> (ø)`
cmd/lotus-miner/proving.go	`27.89% <0.00%> (+0.20%)`	⬆️
cmd/lotus-worker/cli.go	`0.00% <ø> (ø)`
cmd/lotus-worker/info.go	`12.69% <ø> (ø)`
cmd/lotus-worker/main.go	`0.00% <0.00%> (ø)`
cmd/lotus-worker/resources.go	`0.00% <ø> (ø)`
cmd/lotus-worker/storage.go	`0.00% <0.00%> (ø)`
cmd/lotus-worker/tasks.go	`27.27% <ø> (ø)`
extern/sector-storage/selector_task.go	`33.33% <ø> (ø)`
... and 66 more

arajasek

Some notes

extern/sector-storage/ffiwrapper/sealer_cgo.go

extern/sector-storage/manager_post.go

extern/sector-storage/sched_post.go

extern/sector-storage/worker_local.go

Co-authored-by: Aayush Rajasekaran <arajasek94@gmail.com>

…g is set

magik6k · 2022-03-21T19:50:07Z

Just tested in lotus-pond, and it works really well:

Both PoSt types work
Disputer doesn't dispute my PoSts
After restarting lotus-miner, the worker auto-reconnects
When the worker is stopped, lotus-miner switches back to local PoSt automatically

cmd/lotus-miner/info.go

cmd/lotus-seal-worker/main.go

extern/sector-storage/faults.go

ZenGround0 · 2022-03-22T19:31:44Z

extern/sector-storage/faults.go

@@ -161,25 +99,4 @@ func (m *Manager) CheckProvable(ctx context.Context, pp abi.RegisteredPoStProof,
 	return bad, nil
 }

-func addCachePathsForSectorSize(chk map[string]int64, cacheDir string, ssize abi.SectorSize) {
-	switch ssize {


I don't see these stat checks anywhere anymore and no behavior change within GenerateSingleVanillaProof. Did they all become redundant with the check in GenerateSingleVanillaProof at some point?

Yeah, if we can read challanges, even random ones, that probably means that the sector is fine (when challenges are read, they are also validated, at least that's what I've been told)

Will it seriously increase the time spent on check?

ZenGround0 · 2022-03-22T20:20:51Z

extern/sector-storage/storiface/resources.go

+	},
+	sealtasks.TTGenerateWindowPoSt: {
+		abi.RegisteredSealProof_StackedDrg64GiBV1: Resources{
+			MaxMemory: 120 << 30, // TODO: Confirm


Still need confirmation here?

extern/sector-storage/manager_post.go

ZenGround0 · 2022-03-23T16:21:41Z

extern/sector-storage/manager_post.go

+	//  list contains substitutes for skipped sectors - but we don't care about
+	//  those for the purpose of the proof, so for things to work, we need to
+	//  dedupe here.
+	sectorInfo = dedupeSectorInfo(sectorInfo)


I don't think anything like this currently exists. Is this new logic fixing a bug / bugs in current lotus?

ZenGround0 · 2022-03-23T16:25:41Z

extern/sector-storage/manager_post.go

+	sectorInfo = dedupeSectorInfo(sectorInfo)
+
+	// The partitions number of this batch
+	// ceil(sectorInfos / maxPartitionSize)


This is surprising to me. If we had two partitions of size maxPartitionSize and half of each was skipped this code is telling me that we only need to take one partition proof rather than two partition proofs. There's no problem shifting sectors across partitions like this?

extern/sector-storage/manager_post.go

extern/sector-storage/worker_local.go

ZenGround0 · 2022-03-24T17:16:17Z

@magik6k since we're using a different pathway through ffi to do post in the case a worker is hooked up it seems like lotus-bench should adapt to be able to exercise the new path. I'm not sure whether the typical lotus-bench use case is 1) exercise the operator's specific setup or 2) exercise general purpose proving pathways on the miner.

If its 1) we could hook up lotus-bench to the manager so it can talk to workers. If its 2) I think we should add a lotus-bench check that does the challenge -> vanilla proof -> snark ( -> merge multi partition?)

This may be overkill because ffi is doing similar things inside for each. What do you think?

ZenGround0 · 2022-03-24T18:40:17Z

extern/sector-storage/stores/remote_test.go

@@ -31,15 +31,15 @@ import (

 const metaFile = "sectorstore.json"



nit: add test of new method GenerateSingleVanillaProof

extern/sector-storage/stores/remote.go

extern/sector-storage/worker_local.go

ZenGround0

The question about lotus bench remains and I didn't review the new worker itest code too carefully but I'm happy with this going in as is.

ZenGround0 · 2022-03-25T17:47:09Z

extern/sector-storage/sched_post.go

+		if storiface.WorkerID(curSes) != wid {
+			if curSes != ClosedWorkerID {
+				// worker restarted
+				log.Warnw("worker session changed (worker restarted?)", "initial", wid, "current", curSes)


disable here? or is the delete on return enough?

Yes, that delete is enough

magik6k · 2022-03-26T00:23:26Z

On lotus-bench + setting good resource numbers - I think this can be done after this lands in master; I'd love to stop making this PR bigger, especially given that it seems to work pretty well already

(opened #8373 with some in-progress work on top of this PR)

arajasek

👍 on the last five commits

lbj2004032 · 2022-05-16T03:31:44Z

What would happen when miner process exit?
I thought minners prefer a framework that wndpost as a standalone process,
does wndpost/winpost run seperate from minner?

BBQFIL · 2022-09-25T05:49:41Z

@magik6k
Hello, I want to ask how miner judges that a post worker has been closed.
I encountered some problems today. When one of my post workers lost connection to the miner due to network card failure, the miner still handed over the partition to the lost post worker for processing. This is not in line with my expectations. In my opinion, when a post worker fails to handle work, miner should assign tasks to other healthy post workers.

mz-sirius and others added 17 commits January 5, 2022 01:50

decoupling winningpost and windowpost from lotus-miner

3fd55fa

update ffi

6867f01

fix ci err

793b5c7

Merge branch 'feat/decoupling-post-worker' of github.com:wusirdcenter…

cf7dd36

…/lotus into feat/decoupling-post-worker

fix make gen

e216aef

post workers: Cleanup, tests

4a874ef

Merge remote-tracking branch 'origin/master' into feat/post-worker

0c062f9

post workers: Snapdeals updates

f2496d8

gen

f41dd4f

Cleanup imports

03fa955

Untangle ffi from api

b381416

post workers: Fix lint

c79f2b9

docsgen

98fed00

Fix double unlock in WorkerStats

8ac2030

post workers: Share resource accounting code

fa09b9a

don't defer in a loop

b115e28

Merge remote-tracking branch 'origin/master' into feat/post-worker

e476cf7

magik6k marked this pull request as ready for review January 20, 2022 14:03

magik6k requested a review from a team as a code owner January 20, 2022 14:03

stores: Fix single post proof reading with updated sectors

33dfcf2

magik6k force-pushed the feat/post-worker branch from ae4f219 to 33dfcf2 Compare January 20, 2022 14:27

arajasek reviewed Jan 21, 2022

View reviewed changes

magik6k and others added 7 commits January 21, 2022 10:02

Typos

1e5e69e

Co-authored-by: Aayush Rajasekaran <arajasek94@gmail.com>

Address review

4682e72

post workers: Fix skipped handling

82c9e72

post workers: Fix race in setting vproofs

f148397

post workers: Test skips

55c9271

Merge remote-tracking branch 'origin/master' into feat/post-worker

98f4d37

fix some races

efdb854

magik6k force-pushed the feat/post-worker branch from 7051e20 to cca69a6 Compare March 21, 2022 09:49

magik6k added 2 commits March 21, 2022 12:09

Merge remote-tracking branch 'origin/master' into feat/post-worker

92e4caa

lotus-worker: default non-exclusive flags to false when exclusive fla…

56b97e1

…g is set

ZenGround0 reviewed Mar 22, 2022

View reviewed changes

ZenGround0 reviewed Mar 23, 2022

View reviewed changes

This was referenced Mar 24, 2022

Dedicated PoSt workers #5339

Closed

Suspend sealing when proof deadlines are coming up #3792

Closed

magik6k added 4 commits March 24, 2022 18:28

workers: WorkerType mehdod on SealTask

a53dc5a

workers: cmd/lotus-seal-worker->cmd/lotus-worker

3f152ae

wdpost: Lock update files in snapped sectors

13701c7

workers: PoSt challenge throttle/timeout config

0b3144f

ZenGround0 reviewed Mar 25, 2022

View reviewed changes

ZenGround0 approved these changes Mar 25, 2022

View reviewed changes

magik6k added 4 commits March 25, 2022 15:37

stores: log 404 in remote.GenerateSingleVanillaProof

69e8516

worker: Fix challengeThrottle

f1ddb55

cli: Update docs

fcaad20

Merge remote-tracking branch 'origin/master' into feat/post-worker

82343a0

magik6k force-pushed the feat/post-worker branch from b6a3c7f to 82343a0 Compare March 25, 2022 20:49

sealing: Fix TaskType.WorkerType

7a009ab

magik6k mentioned this pull request Mar 26, 2022

feat: bench: simple sealing operations commands #8373

Merged

13 tasks

arajasek approved these changes Mar 26, 2022

View reviewed changes

magik6k merged commit 7401fa2 into master Mar 26, 2022

magik6k deleted the feat/post-worker branch March 26, 2022 00:31

magik6k mentioned this pull request Mar 26, 2022

decoupling post worker #7600

Merged

rjan90 mentioned this pull request Mar 31, 2022

Support reading PoSt challenges from remote storage #5338

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: PoSt workers #7971

feat: PoSt workers #7971

magik6k commented Jan 18, 2022 •

edited

Loading

codecov bot commented Jan 20, 2022 •

edited

Loading

arajasek left a comment

magik6k commented Mar 21, 2022

ZenGround0 Mar 22, 2022

magik6k Mar 24, 2022

yhio Mar 30, 2022

ZenGround0 Mar 22, 2022

magik6k Mar 24, 2022

ZenGround0 Mar 23, 2022

ZenGround0 Mar 23, 2022

ZenGround0 commented Mar 24, 2022

ZenGround0 Mar 24, 2022

ZenGround0 left a comment

ZenGround0 Mar 25, 2022

magik6k Mar 25, 2022

magik6k commented Mar 26, 2022

arajasek left a comment

lbj2004032 commented May 16, 2022

BBQFIL commented Sep 25, 2022

		@@ -31,15 +31,15 @@ import (

		const metaFile = "sectorstore.json"

feat: PoSt workers #7971

feat: PoSt workers #7971

Conversation

magik6k commented Jan 18, 2022 • edited Loading

Related Issues

Proposed Changes

Additional Info

Usage

Followups

Checklist

codecov bot commented Jan 20, 2022 • edited Loading

Codecov Report

arajasek left a comment

Choose a reason for hiding this comment

magik6k commented Mar 21, 2022

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

ZenGround0 commented Mar 24, 2022

Choose a reason for hiding this comment

ZenGround0 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

magik6k commented Mar 26, 2022

arajasek left a comment

Choose a reason for hiding this comment

lbj2004032 commented May 16, 2022

BBQFIL commented Sep 25, 2022

magik6k commented Jan 18, 2022 •

edited

Loading

codecov bot commented Jan 20, 2022 •

edited

Loading