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

Precommit batch balancer support/config #7410

Merged
merged 5 commits into from
Oct 1, 2021
Merged

Precommit batch balancer support/config #7410

merged 5 commits into from
Oct 1, 2021

Conversation

magik6k
Copy link
Contributor

@magik6k magik6k commented Sep 30, 2021 •
edited
Loading

Implements #7391

@magik6k magik6k requested a review from arajasek September 30, 2021 14:56
@magik6k magik6k requested a review from a team as a code owner September 30, 2021 14:56
@magik6k magik6k added this to the Network v14 milestone Sep 30, 2021
@magik6k magik6k linked an issue Sep 30, 2021 that may be closed by this pull request
Implement the BatchBalancer for precommitbatch #7391
Closed
1 task
arajasek
arajasek reviewed Sep 30, 2021
View reviewed changes
extern/storage-sealing/precommit_batch.go Outdated Show resolved Hide resolved
extern/storage-sealing/precommit_batch.go Outdated Show resolved Hide resolved
extern/storage-sealing/precommit_batch.go Outdated
return nil, xerrors.Errorf("couldn't get base fee: %w", err)
}

if bf.LessThan(cfg.BatchPreCommitAboveBaseFee) { // todo: only enable after nv14?
Copy link
Contributor

Choose a reason for hiding this comment

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

This is probably a must have, yeah

@codecov
Copy link

codecov bot commented Oct 1, 2021 •
edited
Loading

Codecov Report

Merging #7410 (a2aed6c) into feat/nv14 (89d61ca) will increase coverage by 0.05%.
The diff coverage is 64.28%.

❗ Current head a2aed6c differs from pull request most recent head 165d494. Consider uploading reports for the commit 165d494 to get more accurate results
Impacted file tree graph

@@              Coverage Diff              @@
##           feat/nv14    #7410      +/-   ##
=============================================
+ Coverage      39.28%   39.34%   +0.05%     
=============================================
  Files            627      627              
  Lines          66248    66169      -79     
=============================================
+ Hits           26028    26033       +5     
+ Misses         35713    35647      -66     
+ Partials        4507     4489      -18
Impacted Files Coverage Δ
extern/storage-sealing/precommit_batch.go 67.19% <49.15%> (-5.63%) ⬇️
...rn/storage-sealing/mocks/mock_precommit_batcher.go 82.69% <100.00%> (+3.62%) ⬆️
node/config/def.go 98.64% <100.00%> (+<0.01%) ⬆️
node/modules/storageminer.go 62.50% <100.00%> (+0.12%) ⬆️
extern/sector-storage/sched_resources.go 75.00% <0.00%> (-9.38%) ⬇️
chain/actors/builtin/paych/paych.go 19.31% <0.00%> (-4.55%) ⬇️
chain/actors/builtin/market/market.go 59.74% <0.00%> (-4.48%) ⬇️
chain/stmgr/execute.go 86.95% <0.00%> (-4.35%) ⬇️
markets/storageadapter/ondealsectorcommitted.go 77.33% <0.00%> (-4.00%) ⬇️
extern/storage-sealing/mocks/api.go 17.84% <0.00%> (-3.74%) ⬇️
... and 41 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 89d61ca...165d494. Read the comment docs.

@arajasek
Copy link
Contributor

arajasek commented Oct 1, 2021

Almost good to go, needs some work once filecoin-project/specs-actors#1497 lands

@jennijuju
Copy link
Member

should we take this chance to lower PreCommitBatchWait? SPs have been complaining this is too long

@arajasek
Copy link
Contributor

arajasek commented Oct 1, 2021

@jennijuju Sure, but I dunno what we want...Wanna push a commit to this branch / gimme a new value?

@jennijuju
Copy link
Member

@jennijuju Sure, but I dunno what we want...Wanna push a commit to this branch / gimme a new value?

we can ignore yesterday-me

@arajasek arajasek force-pushed the feat/nv14-pc-balancer branch 2 times, most recently from 165d494 to 1475b31 Compare October 1, 2021 18:59
@arajasek arajasek force-pushed the feat/nv14-pc-balancer branch from 1475b31 to a335cb3 Compare October 1, 2021 21:11
@arajasek arajasek merged commit 207746f into feat/nv14 Oct 1, 2021
@arajasek arajasek deleted the feat/nv14-pc-balancer branch October 1, 2021 21:23
@BlocksOnAChain BlocksOnAChain mentioned this pull request Oct 4, 2021
Closed
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

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Network v14
Development

Successfully merging this pull request may close these issues.

Implement the BatchBalancer for precommitbatch
3 participants
@magik6k @arajasek @jennijuju