Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SR2 miner selector & safer default StorageConfig config #614

Merged
merged 15 commits into from
Sep 26, 2020
Merged

Conversation

jsign
Copy link
Contributor

@jsign jsign commented Sep 24, 2020

This PR creates a new MinerSelector implementation, which executes the planned strategy for selecting miners in SR2.
In a nutshell, when this miner selector is used:

  • A remote .json file is automatically fetched and parsed which contains which miners are going to be used to make deals.
  • An example of that file is here
  • What Powergate will do is to select Amount random miners from each miners bucket.

This allows to:

  • Update the miner list in github.com/filecoin-project/slingshot repository, and automatically update every running Powerate without restarts. Every time a list of miners needs to be selected, it will fetch the current list.json in the master branch of that repo.
  • If we think is a good/bad idea to increase/decrease the replication factor, it can be easily done by increase/decreasing the Amount attribute of each miners bucket in the json file.

By default now we configure Poweragate to use this miner selector strategy. A new flag was included if someone wants to keep using the previous one; so at the end of the day is configurable. The URL where this JSON will be fetched is also configurable via flags/envs, so if someone wants to use their own JSON file, that's OK too.

So in short, the default now is to have the right configuration for the SR2 contest.

Some other lateral changes:

  • The default StorageConfig has HotStorage disabled as a more reasonable configuration to avoid disk usage for SR2.
  • The AddTimeout is hardcoded to a bigger value ignoring the StorageConfig. This is a temporal solution until we have a bit more control to make general StorageConfig changes in the Hub.
  • lotus-devnet was updated to v0.7.2.
  • Default deal duration decreased to 6 months as decided today in SR2 call.
  • Keep using reputation minerselector, since SR2 want to hold a bit using the new one created in this PR.

@jsign jsign added this to the Sprint 46 milestone Sep 24, 2020
@jsign jsign self-assigned this Sep 24, 2020
var ms ffs.MinerSelector
var err error

switch conf.MinerSelector {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MinerSelector configuration depending on flags/envs.

cmd/powd/main.go Outdated
@@ -301,6 +308,8 @@ func setupFlags() error {
pflag.String("maxminddbfolder", ".", "Path of the folder containing GeoLite2-City.mmdb")
pflag.String("mongouri", "", "Mongo URI to connect to MongoDB database. (Optional: if empty, will use Badger)")
pflag.String("mongodb", "", "Mongo database name. (if --mongouri is used, is mandatory")
pflag.String("ffsminerselector", "sr2", "Miner selector to be used by FFS: 'sr2', 'reputation'")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MinerSelector can be:

  • sr2: the default now. Does what was commented in PR description.
  • reputation: The previous miner selector that uses the reputation index, and still directly targets peter's miners as trusted.

Enabled: true,
Enabled: false,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decided with @asutula to avoid disk usage explosion.
If hte user want's HotStorage enabled, should be set explicitly on pushing a config.

@@ -0,0 +1,143 @@
package sr2
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary of the selector:

  • Fetch the json file
  • Parse it
  • Get covered on some possibly weird values (amount < 0, etc)
  • Use the ask cache index as much as possible. Some miners might not be there if they have 0 power. So instead of changing the ask index to include also miners with 0 power (which are a lot and would affect index building times), I decided that if a miner in this trusted list hasn't a cached ask, we'd do that directly here.

}

// GetMiners returns miners from SR2.
func (ms *MinerSelector) GetMiners(n int, f ffs.MinerSelectorFilter) ([]ffs.MinerProposal, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important fact. This MinerSelector will ignore n and f.
The replication factor will be the sum of Amount of buckets, and not filtering is done.

So this is a very aggressive miner selector kind of forcing what was decided in SR2. No custom StorageConfig will change this fact, unless the user runs with --ffsminerselector=reputation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't remember, but I suppose the replication factor provided in the storage config is passed into a miner selector via ffs.MinerSelectorFilter? So this means that user provided replication factor is ignored? That is fine, just want to be sure I understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, is ignored. Now will be controled in this remote file now that --ffsminerselector=sr2 default miner selector.
If someone wants to come back to not being ignored, they could specify --ffsminerselector=reputation and things will work as before.

@jsign jsign changed the title SR2 miner selector SR2 miner selector & safer default StorageConfig config Sep 24, 2020
Comment on lines 205 to 207
// ToDo: this is a hot-fix to force a big timeout until we have a
// migration tool to make this tunable again.
sctx, cancel := context.WithTimeout(ctx, time.Second*300)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See this.

@jsign jsign requested a review from asutula September 24, 2020 15:34
@jsign jsign marked this pull request as ready for review September 24, 2020 15:34
Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
Copy link
Member

@asutula asutula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. Just one question for my own understanding.

}

// GetMiners returns miners from SR2.
func (ms *MinerSelector) GetMiners(n int, f ffs.MinerSelectorFilter) ([]ffs.MinerProposal, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't remember, but I suppose the replication factor provided in the storage config is passed into a miner selector via ffs.MinerSelectorFilter? So this means that user provided replication factor is ignored? That is fine, just want to be sure I understand.

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
@jsign jsign merged commit dfaf944 into master Sep 26, 2020
@jsign jsign deleted the jsign/sr2ms branch September 26, 2020 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants