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

Archive storage config API #361

Merged
merged 14 commits into from
Sep 28, 2020
Merged

Archive storage config API #361

merged 14 commits into from
Sep 28, 2020

Conversation

asutula
Copy link
Member

@asutula asutula commented Sep 24, 2020

Allows getting and setting the default ArchiveConfig per Bucket and allows you to pass an override ArchiveConfig with each call to Archive.

Updates everything from mongo, rpc, client, local buckets api, and cli.

closes #336
closes #339

@asutula asutula self-assigned this Sep 24, 2020
api/buckets/client/client.go Show resolved Hide resolved
api/buckets/pb/buckets.proto Show resolved Hide resolved
api/buckets/pb/buckets.proto Show resolved Hide resolved
api/buckets/service.go Show resolved Hide resolved
api/buckets/service.go Show resolved Hide resolved
api/buckets/service.go Outdated Show resolved Hide resolved
buckets/local/archive.go Show resolved Hide resolved
buckets/local/archive.go Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
mongodb/bucketarchives.go Show resolved Hide resolved
Signed-off-by: Aaron Sutula <[email protected]>
@asutula asutula requested review from sanderpick and jsign September 25, 2020 22:47
@asutula asutula marked this pull request as ready for review September 25, 2020 22:47
@asutula
Copy link
Member Author

asutula commented Sep 26, 2020

Ok getting a legit error in the integration test TestArchiveBucketWorkflow. The error is:

executing cold-storage config: making deal configs: getting miners from minerselector: not enough miners that satisfy the constraints

@jsign just checking if there is something we need to change about the lotus devnet and/or powergate setup in here? Anything come to mind?

@asutula
Copy link
Member Author

asutula commented Sep 26, 2020

Updated the compose file to powergate 0.6.6 and lotus-devnet sha-35afaa3 and now the error is:

executing cold-storage config: all proposals were rejected

@jsign
Copy link
Contributor

jsign commented Sep 26, 2020

Ok getting a legit error in the integration test TestArchiveBucketWorkflow. The error is:

executing cold-storage config: making deal configs: getting miners from minerselector: not enough miners that satisfy the constraints

@jsign just checking if there is something we need to change about the lotus devnet and/or powergate setup in here? Anything come to mind?

Yes, most probably is related to forcing Peter's miners in the version of Powergate you've updated. Now that in v0.6.6 we switched to SR2 miner selector by default, might make sense again to have the original StorageConfig which doesn't have a replication factor of 5, which is my best guess about that error.
So the idea would be:

  • Revert to the original StorageConfig with rep facto 1 and without trusted miners.
  • Set explicitly for tests here to use --ffsminerselector=reputation, since now the default is sr2.

I can confirm this theory on Monday, but makes sense to me.
BTW, might require a new tag in Powergate and bubble up here.

@asutula
Copy link
Member Author

asutula commented Sep 26, 2020

Thanks @jsign. That gives me a path forward. Will dig in more later.

Copy link
Contributor

@jsign jsign left a comment

Choose a reason for hiding this comment

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

LGTM! I left some questions in the wild.

api/buckets/pb/buckets.proto Show resolved Hide resolved
api/buckets/pb/buckets.proto Show resolved Hide resolved
api/buckets/service.go Outdated Show resolved Hide resolved
api/buckets/service.go Outdated Show resolved Hide resolved
api/buckets/service.go Show resolved Hide resolved
api/buckets/service.go Show resolved Hide resolved
api/buckets/service.go Outdated Show resolved Hide resolved
api/buckets/service.go Show resolved Hide resolved
integrationtest/pg/pg_test.go Outdated Show resolved Hide resolved
integrationtest/pg/powcli/docker-compose-pow.yml Outdated Show resolved Hide resolved
Copy link
Member

@sanderpick sanderpick left a comment

Choose a reason for hiding this comment

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

LGTM! I left a bunch of nits, but otherwise looks solid.

api/buckets/client/client.go Show resolved Hide resolved
api/buckets/client/client.go Show resolved Hide resolved
api/buckets/client/options.go Show resolved Hide resolved
api/buckets/service.go Outdated Show resolved Hide resolved
buckets/local/archive.go Show resolved Hide resolved
buckets/local/archive.go Show resolved Hide resolved
buckets/local/archive.go Show resolved Hide resolved
buckets/local/archive.go Outdated Show resolved Hide resolved
integrationtest/pg/docker-compose.yml Show resolved Hide resolved
cmd/buck/cli/archive.go Show resolved Hide resolved
Signed-off-by: Aaron Sutula <[email protected]>
Signed-off-by: Aaron Sutula <[email protected]>
Signed-off-by: Aaron Sutula <[email protected]>
Signed-off-by: Aaron Sutula <[email protected]>
@asutula asutula merged commit 2a72b54 into master Sep 28, 2020
@asutula asutula deleted the asutula/bucket-archive-config branch September 28, 2020 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add per bucket archive storage config option Add default storage config to bucket archive api
3 participants