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.

Sign up for GitHub

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

Jump to bottom

Configurable CC Sector Expiration #6803

Merged
merged 9 commits into from
Jul 28, 2021
Merged

Configurable CC Sector Expiration #6803

merged 9 commits into from
Jul 28, 2021

Conversation

placer14
Copy link
Contributor

@placer14 placer14 commented Jul 20, 2021 •
edited
Loading

(fixes #6836)

Adds config:

 	// CommittedCapacitySectorLifetime is the duration a Committed Capacity (CC) sector will
	// live before it must be extended or converted into sector containing deals before it is
	// terminated. Value must be between 180-540 days inclusive
	CommittedCapacitySectorLifetime Duration

Related #4760

@placer14 placer14 force-pushed the mg/feat/configurable-sector-expirations branch from e7d9b33 to a647722 Compare July 20, 2021 21:27
placer14
placer14 commented Jul 20, 2021
View reviewed changes
node/config/def.go Outdated Show resolved Hide resolved
magik6k
magik6k reviewed Jul 21, 2021
View reviewed changes
storage/miner.go Outdated Show resolved Hide resolved
node/config/def.go Outdated Show resolved Hide resolved
@placer14 placer14 changed the title Configurable Sector Expiration (WIP) Configurable CC Sector Expiration (WIP) Jul 22, 2021
@placer14 placer14 force-pushed the mg/feat/configurable-sector-expirations branch from f47bdb2 to 0085fc0 Compare July 22, 2021 12:39
@placer14
Copy link
Contributor Author

Rebased onto latest master branch. ^

@placer14 placer14 mentioned this pull request Jul 22, 2021
Open
2 tasks
@placer14 placer14 force-pushed the mg/feat/configurable-sector-expirations branch from 0085fc0 to 973653f Compare July 22, 2021 12:48
@placer14 placer14 force-pushed the mg/feat/configurable-sector-expirations branch from 5e617c5 to 9e08b46 Compare July 22, 2021 19:07
@placer14 placer14 force-pushed the mg/feat/configurable-sector-expirations branch from 9e08b46 to 3829d6b Compare July 22, 2021 19:10
@placer14 placer14 marked this pull request as ready for review July 22, 2021 19:28
@placer14 placer14 requested a review from a team as a code owner July 22, 2021 19:28
@placer14 placer14 requested a review from magik6k July 22, 2021 19:28
placer14
placer14 commented Jul 22, 2021
View reviewed changes
extern/storage-sealing/precommit_policy_test.go
// as set just before returning within Expiration()
expected += miner.WPoStProvingPeriod - (expected % miner.WPoStProvingPeriod) + pBoundary - 1
assert.Equal(t, int(expected), int(exp))
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The math is a little obtuse here so I put literal expecteds for both paths (zero config for CC and non-CC sectors) as well as show how the custom value is applied. I kept the later units with literals in the assert and simply updated the values.

placer14
placer14 commented Jul 22, 2021
View reviewed changes
extern/storage-sealing/precommit_policy_test.go
expected := h + customLifetimeEpochs - (pBuffer * 2)
// as set just before returning within Expiration()
expected += miner.WPoStProvingPeriod - (expected % miner.WPoStProvingPeriod) + pBoundary - 1
assert.Equal(t, int(expected), int(exp))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm pretty sure this math is unchanged, but please double-check this against the impl. 🙇

@placer14 placer14 changed the title Configurable CC Sector Expiration (WIP) Configurable CC Sector Expiration Jul 22, 2021
magik6k
magik6k reviewed Jul 23, 2021
View reviewed changes
Copy link
Contributor

@magik6k magik6k left a comment

Choose a reason for hiding this comment

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

Looks good, just one tiny issue

storage/miner.go Show resolved Hide resolved
extern/storage-sealing/precommit_policy.go Outdated Show resolved Hide resolved
extern/storage-sealing/precommit_policy.go Show resolved Hide resolved
@placer14
fix: Remove unnecessary Truncate
41bce79
@placer14
Merge branch 'origin/master' into mg/feat/configurable-sector-expirat…
64d2b7a 
…ions

* origin/master: (47 commits)
  Fix links in issue templates
  Update issue templates and add templates for M1
  config: Add newlines consistently
  config: Add note on codegen to types.go
  fix lint
  cli docsgen
  config: Bring some docs from filecoin-docs
  add comment about the necessity of FileIO
  improve README
  config doc gen: generate formatted code
  config: Fix doc finding logic
  Nicer lotus-miner config commands
  Move doc-comment logic to the config pkg
  config: doc struct codegen
  lotus daemon config utils
  adjust compaction test with badger to hit the db
  add splitstore compaction test with badger markset
  update README with configuration options
  tweak badger options
  some tweaks in badger markset
  ...

Conflicts:
        node/config/def.go
@placer14
fix: Update node config docs
e68c59f
@placer14 placer14 requested a review from magik6k July 26, 2021 15:09
@placer14
Copy link
Contributor Author

@magik6k This should be ready to merge, despite some testground PRs which are hanging. I've kicked them off again to see if they'll resolve.

@jennijuju jennijuju linked an issue Jul 26, 2021 that may be closed by this pull request
Allow miners to set expiration duration for (deal) sectors #4760
Open
2 tasks
@BigLep BigLep requested a review from a team July 26, 2021 22:46
@arajasek arajasek enabled auto-merge July 28, 2021 20:45
@arajasek arajasek merged commit 4a90c0f into master Jul 28, 2021
@arajasek arajasek deleted the mg/feat/configurable-sector-expirations branch July 28, 2021 21:07
@jennijuju jennijuju mentioned this pull request Aug 9, 2021
Open
@jennijuju jennijuju mentioned this pull request Sep 6, 2021
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arajasek arajasek arajasek approved these changes

@magik6k magik6k Awaiting requested review from magik6k

Assignees

@placer14 placer14

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow miners to set expiration duration for (deal) sectors default expiration duration for cc sectors
3 participants
@placer14 @magik6k @arajasek