Skip to content

bucket archive improvements and limits#457

Merged
asutula merged 4 commits intomasterfrom
asutula/bucket-archive-nits
Dec 21, 2020
Merged

bucket archive improvements and limits#457
asutula merged 4 commits intomasterfrom
asutula/bucket-archive-nits

Conversation

@asutula
Copy link
Member

@asutula asutula commented Dec 21, 2020

  • Configurable limit to the max rep factor for bucket archives, defaults to 3
  • Don't expose address as part of archive config... just use the user's only address (we don't allow users to create additional addresses)
  • Better json representation of ArchiveConfig that is consistent with the JS client. Makes json ArchiveConfig data interchangeable and consistent between the two clients.

Signed-off-by: Aaron Sutula <hi@asutula.com>
@asutula asutula requested a review from jsign December 21, 2020 17:26
@asutula asutula self-assigned this Dec 21, 2020
repeated string trusted_miners = 4;
repeated string country_codes = 5;
ArchiveRenew renew = 6;
string addr = 7;
Copy link
Member Author

Choose a reason for hiding this comment

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

No more address in the api

ArchiveTracker *archive.Tracker
Semaphores *nutil.SemaphorePool
MaxBucketSize int64
MaxBucketArchiveRepFactor int
Copy link
Member Author

Choose a reason for hiding this comment

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

This gets set via configuration

storageConfig.Cold.Filecoin.Address = defConfRes.DefaultStorageConfig.Cold.Filecoin.Address
}
// Get the address from the default storage config for this user.
storageConfig.Cold.Filecoin.Address = defConfRes.DefaultStorageConfig.Cold.Filecoin.Address
Copy link
Member Author

Choose a reason for hiding this comment

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

We now always get the address from the user's default Powergate StorageConfig, they can't modify this via ArchiveConfig anymore.

}
}

func (s *Service) validateArchiveConfig(c *mdb.ArchiveConfig) error {
Copy link
Member Author

Choose a reason for hiding this comment

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

Simple validation of an ArchiveConfig

// Threshold indicates how many epochs before expiring should trigger
// deal renewal. e.g: 100 epoch before expiring.
Threshold int
Threshold int `json:"threshold"`
Copy link
Member Author

Choose a reason for hiding this comment

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

Normal json formatting that is also consistent with the JS client.

// Renew indicates deal-renewal configuration.
Renew ArchiveRenew `bson:"renew"`
// Addr is the wallet address used to store the data in filecoin
Addr string `bson:"addr"`
Copy link
Member Author

Choose a reason for hiding this comment

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

Previously serialized address values will just be ignored from here on out.

Signed-off-by: Aaron Sutula <hi@asutula.com>
@asutula
Copy link
Member Author

asutula commented Dec 21, 2020

I added the rep factor limit here in Hub, but also maybe we should add that to Powergate?

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.

Considering it has also breaking API changes, this PR should fire some major version bump process?

@jsign
Copy link
Contributor

jsign commented Dec 21, 2020

I added the rep factor limit here in Hub, but also maybe we should add that to Powergate?

Sounds like a good safety net and potentially useful for people running Powergate alone.
I wouldn't rush to implement it, but sounds like a good idea.. or maybe it's just an application thing and not worth doing it.

Signed-off-by: Aaron Sutula <hi@asutula.com>
Signed-off-by: Aaron Sutula <hi@asutula.com>
@asutula asutula merged commit 4149a93 into master Dec 21, 2020
@asutula asutula deleted the asutula/bucket-archive-nits branch December 21, 2020 19:40
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.

2 participants