Support standalone miner-market process #6356

nonsense · 2021-05-31T09:38:46Z

Associated docs: filecoin-project/filecoin-docs#890

This PR is a cherry-picked version of #5983. Closes #5149.

It introduces the concept of Subsystems within the lotus-miner process - currently:

StorageMarket
Mining
Sealing
SectorStorage

It adds support for running the miner market module as a separate process.
A lotus-miner market service connects to the main lotus process for all sealing/storage interactions, or:

market service runs StorageMarget subsystem
main miner service runs Mining, Sealing and SectorStorage subsystems.

Depending on configuration options, this PR provides flexibility for the the miner to be split into multiple processes, or to be run as one process as is currently the case.

Currently the deployment flow for running a market module as a separate process is the following:

Create backup of the original miner repo / metadata (i.e. something like: export LOTUS_BACKUP_BASE_PATH=~/.lotusbackup && export LOTUS_MINER_PATH=~/.lotusminer && export LOTUS_PATH=~/.devlotus && lotus-miner backup ~/.lotusbackup/backupfile)
Initialise the market service based on the backup from 1, and generate a new libp2p identity (i.e. something like:

APISEALER=`./lotus-miner auth api-info --perm=admin`

export LOTUS_PATH=~/.devLotus && export LOTUS_MINER_PATH=~/.restoredrepo && ./lotus-miner init service --enable-market --api-sealer=$APISEALER --api-sector-index=$APISEALER --config=~/.lotusmarket/config.toml --storage-config=~/.lotusmarket/storage.json ~/.lotusbackup/backupfile

This would also send a message on chain and update the address for the given miner, so that the markets process is the point of entry from the on-chain DHT.
3. Run market service as well as main miner service (i.e. something like:

./lotus-miner run --nosync
LOTUS_MINER_PATH=~/.restoredrepo ./lotus-miner run --nosync

TODO:

update documentation - initial docs for splitting miner and markets service filecoin-docs#890
resolve conflicts with dynamic pricing PR
resolve conflicts and update tests based on itests/kit
fix unit tests

TODO FROM PARENT:

FOLLOW UP / NICE TO HAVE:

Add Testground testplan, so that we support both mode of operation - separate processes for miner and markets, as well as one process for miner and markets
Return nicer errors from unsupported api calls when subsystems are disabled
- Not sure if we can do much better than if x == nil { error(module x is disabled) }
Improve dev experience with respect to setup

…model moved to api package - PieceDealInfo, DealSchedule

magik6k · 2021-05-31T10:53:47Z

This will likely conflict with #6280 a bunch (it also cherry-picks from my PR)

…ings

…ng miner

magik6k

Looks almost mergable, just a couple of small comments

magik6k · 2021-07-07T13:22:31Z

node/builder.go

@@ -242,7 +243,14 @@ func ConfigCommon(cfg *config.Common) Option {
 			urls = append(urls, "http://"+ip+"/remote") // TODO: This makes no assumptions, and probably could...
 			return urls, nil
 		}),
-		ApplyIf(func(s *Settings) bool { return s.Online },
+		ApplyIf(func(s *Settings) bool { return s.Base }), // apply only if Base has already been applied


Looking at this now, it probably makes more sense for this to Error if base isn't applied here

extern/sector-storage/stores/remote.go

node/builder.go

magik6k · 2021-07-07T15:19:36Z

node/builder_miner.go

+			Override(new(stores.SectorIndex), From(new(modules.MinerSealingService))),
+		),
+
+		If(cfg.Subsystems.EnableStorageMarket,


Why don't we include the LibP2P option here (and in the client options)? This should allow us to get rid of settings.enableLibp2pNode

I will give this a try, but I am not sure it is possible, as there are some configs that need to be applied earlier.

Agree. Now that we've separated Libp2p from Online, enableLibp2pNode maps directly to enabling the Libp2p aggregate. This should be feasible.

Oh wait, I think this may have to do with the fact that we need to read the Config to know whether to enable or not the libp2p node, right?

Only when Markets is enabled in the miner config, should we enable libp2p. So it's a bit of a chicken and egg, and that's why it's convoluted.

Actually, no. It should be possible, as ConfigStorageMiner and ConfigureFullNode have access to the config, and can decide whether to insert the Libp2p aggregate within the Options they return.

node/modules/storageminer.go

raulk

Initial comments. We probably want to bring consistency between the terms "subsystem" and "service" -- we use them a bit interchangeably?

cmd/lotus-storage-miner/init_service.go

node/config/def.go

node/impl/common/mock/mock.go

raulk · 2021-07-07T18:47:39Z

node/impl/storminer.go

+	PieceStore        dtypes.ProviderPieceStore         `optional:"true"`
+	StorageProvider   storagemarket.StorageProvider     `optional:"true"`
+	RetrievalProvider retrievalmarket.RetrievalProvider `optional:"true"`
+	DataTransfer      dtypes.ProviderDataTransfer       `optional:"true"`
+	DealPublisher     *storageadapter.DealPublisher     `optional:"true"`
+	SectorBlocks      *sectorblocks.SectorBlocks        `optional:"true"`
+	Host              host.Host                         `optional:"true"`


We should move these out to a MarketsAPI struct and embed it here.

node/impl/storminer.go

raulk

LGTM, besides the comments! This was a heroic effort. Awesome work here, @nonsense and @magik6k ❤️ This is the start of a more modular Lotus!

Some top-level comments:

Let's standardise on the name "subsystem" or "service". We use both interchangeably and it's confusing (service in the cli tooling, subsystem in the config). Either is fine with me, unless we do want to use both because they refer to different things now, or we expect them to (e.g. service as a collection of subsystems?). If so, we should document the difference.
OK to merge the changes in the testkit as is. I can follow-up with a new PR to bring more order/structure to that and de-entropise.
We may need commands to query a lotus node to know which subsystems is running, and to know which other APIs it's interacting with, but that can be a follow-up.

extern/sector-storage/manager.go

extern/storage-sealing/input.go

node/modules/storageminer.go

cmd/lotus-storage-miner/init_restore.go

raulk · 2021-07-07T19:21:53Z

cmd/lotus-storage-miner/init_restore.go

-		if err := checkV1ApiSupport(ctx, cctx); err != nil {
-			return err
-		}


We don't need this any longer?

raulk · 2021-07-07T19:24:12Z

cmd/lotus-storage-miner/init_restore.go

-		if cctx.Args().Len() != 1 {
-			return xerrors.Errorf("expected 1 argument")
-		}
+func restore(ctx context.Context, cctx *cli.Context, manageConfig func(*config.StorageMiner) error, after func(api lapi.FullNode, addr address.Address, peerid peer.ID, mi miner.MinerInfo) error) error {


FYI, I wasn't able to review this file because the diff is too noisy and I'm not familiar with this command. I trust @magik6k here.

cmd/lotus-storage-miner/run.go

cmd/lotus-storage-miner/init_service.go

magik6k

Well, I think this is in good enough shape to land in master, just one cosmetic suggestion.

magik6k · 2021-07-12T19:27:07Z

cmd/lotus-storage-miner/init_service.go

-			Name:  "enable-market",
-			Usage: "enable market module",
+		&cli.StringSliceFlag{
+			Name:  "name",


Maybe type? lotus-miner init service --name markets ... looks a bit weild

Renamed to type.

… new service

nonsense · 2021-07-13T11:13:11Z

@magik6k @raulk in the last commit (2af02af), I renamed --name to --type and also addressed the UX problems with storage-config.json:

restore is used both for backup restoration and for initialisation of new services, so it needs to support 3 modes:

restoring from backup where the user wants to restore a given storage-config.json
restoring from backup where the user wants to restore other parts from their backup, and keep their existing storage-config.json
initialising a new markets type service, where we need a blank storage-config.json (with empty set of StoragePaths), since at the moment Remote requires a Local store, which requires a proper config.

So parsing of the storage-config.json is extracted away, and we call restore with the necessary *stores.StorageConfig

Docs at filecoin-project/filecoin-docs#890 have also been addressed to not mention storage-config.json anymore and use --type, so UX is better now for miners who want to split the monolith.

magik6k

⛵

nonsense added 21 commits May 19, 2021 13:05

add SectorAddPieceToAny and SectorUnsealPiece to StorageMiner iface; …

a989f60

…model moved to api package - PieceDealInfo, DealSchedule

extract pushUrl and fix return type for worker RPC v0

87ed228

export PartialFile and OpenPartialFile

e275b54

add UnpaddedByteIndex.Valid()

5540111

move handleProvingSector to correct file - states_proving.go

83e55dc

confirm that Miner struct satisfies sectorblocks.SectorBuilder

1295c92

update sectorstorage.New (Manager) interface

2aad7b6

add ReaderParamDecoder to json rpc server to lotus-miner run

2562f2e

update reader to use ReadAtLeast

c12d802

update retrievaladapter ; add piece_provider

cb603c6

update StorageMinerAPI

76bb424

re-arrange NodeBuilder and add storageminer_svc

4693c61

remove DealInfo and DealSchedule from storage-sealing gen/main.go

9092899

add remoteGetAllocated http handler

1a9b576

update remoteGetSector

19bd5be

comment

9a7b0b6

introduce init service, refactor restore service

8d3d3c8

logs and defaults

9979de2

add missing messagepool override

c311520

bump api ver

46a368d

Merge branch 'master' into nonsense/split-market-miner-processes

d195a12

nonsense added 2 commits June 1, 2021 11:42

add modules.StorageAuthWithURL to set correct token

e4136b0

fix tests

0a79f49

jacobheun added epic/MRA team/ignite Issues and PRs being tracked by Team Ignite at Protocol Labs labels Jun 1, 2021

nonsense added 4 commits June 4, 2021 15:41

fix TestAPIDeal tests

88756f3

make linter happy

7f23480

add messagepool to test node constructor

c877166

resolve merge conflicts

8bd09e3

nonsense added 4 commits July 6, 2021 18:12

fix signature

566a9d5

remove ApplyIfEnableLibP2P

42ae876

variadic WithSubsystems

caa366b

pass Subsystems to StorageMiner option; add enableLibp2p bool in Sett…

6b014f5

…ings

nonsense force-pushed the nonsense/split-market-miner-processes branch from 68ab7b4 to 6b014f5 Compare July 7, 2021 12:15

nonsense added 2 commits July 7, 2021 14:21

refactor testkit in lotus-soup to include Subsystems when instantiati…

1fe08f5

…ng miner

fix cast on role_miner in testground lotus-soup plan

bb466ed

magik6k reviewed Jul 7, 2021

View reviewed changes

fixup

82efe01

raulk reviewed Jul 7, 2021

View reviewed changes

raulk approved these changes Jul 7, 2021

View reviewed changes

nonsense added 6 commits July 12, 2021 11:30

remove pieceProvider from DI; small refactors

16784aa

resolve conflicts

d89ddb9

rename mock.go to net.go

a423c46

rename argument from url to apiInfo

056136e

rename cfg.Subsystems.EnableStorageMarket to EnableMarkets

4bc9fa0

remove --enable-markets flag; add --name string slice flag

7728d6b

nonsense force-pushed the nonsense/split-market-miner-processes branch from c884d81 to 7728d6b Compare July 12, 2021 12:38

magik6k approved these changes Jul 12, 2021

View reviewed changes

rename --name to --type; provide empty stores.StorageConfig when init…

2af02af

… new service

nonsense force-pushed the nonsense/split-market-miner-processes branch from 4299bdd to 2af02af Compare July 13, 2021 10:57

magik6k approved these changes Jul 13, 2021

View reviewed changes

magik6k merged commit 837322e into master Jul 13, 2021

magik6k deleted the nonsense/split-market-miner-processes branch July 13, 2021 15:16

jennijuju added M1-release and removed epic/MRA labels Jul 19, 2021

nonsense mentioned this pull request Jul 21, 2021

add SealProof in SectorBuilder #6815

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support standalone miner-market process #6356

Support standalone miner-market process #6356

nonsense commented May 31, 2021 •

edited by raulk

Loading

magik6k commented May 31, 2021

magik6k left a comment

magik6k Jul 7, 2021

magik6k Jul 7, 2021

nonsense Jul 7, 2021

raulk Jul 7, 2021

raulk Jul 7, 2021

raulk Jul 7, 2021

raulk Jul 7, 2021 •

edited

Loading

raulk left a comment

raulk Jul 7, 2021

raulk left a comment

raulk Jul 7, 2021

raulk Jul 7, 2021

magik6k left a comment

magik6k Jul 12, 2021

nonsense Jul 13, 2021

nonsense commented Jul 13, 2021 •

edited

Loading

magik6k left a comment

Support standalone miner-market process #6356

Support standalone miner-market process #6356

Conversation

nonsense commented May 31, 2021 • edited by raulk Loading

magik6k commented May 31, 2021

magik6k left a 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

raulk Jul 7, 2021 • edited Loading

Choose a reason for hiding this comment

raulk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raulk 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 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nonsense commented Jul 13, 2021 • edited Loading

magik6k left a comment

Choose a reason for hiding this comment

nonsense commented May 31, 2021 •

edited by raulk

Loading

raulk Jul 7, 2021 •

edited

Loading

nonsense commented Jul 13, 2021 •

edited

Loading