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

feat: proving: Introduce manual sector fault recovery #9144

Merged
merged 14 commits into from
Sep 6, 2022
Merged

feat: proving: Introduce manual sector fault recovery #9144

merged 14 commits into from
Sep 6, 2022

Conversation

LexLuthr
Copy link
Contributor

@LexLuthr LexLuthr commented Aug 9, 2022

Related Issues

#7748

Proposed Changes

Allow users to declare fault recovery manually rather than waiting for windowPost.

Additional Info

Checklist

Before you mark the PR ready for review, please make sure that:

  • All commits have a clear commit message.
  • The PR title is in the form of of <PR type>: <area>: <change being made>
    • example: fix: mempool: Introduce a cache for valid signatures
    • PR type: fix, feat, INTERFACE BREAKING CHANGE, CONSENSUS BREAKING, build, chore, ci, docs,perf, refactor, revert, style, test
    • area: api, chain, state, vm, data transfer, market, mempool, message, block production, multisig, networking, paychan, proving, sealing, wallet, deps
  • This PR has tests for new functionality or change in behaviour
  • If new user-facing features are introduced, clear usage guidelines and / or documentation updates should be included in https://lotus.filecoin.io or Discussion Tutorials.
  • CI is green

@LexLuthr LexLuthr linked an issue Aug 9, 2022 that may be closed by this pull request
14 tasks
@LexLuthr LexLuthr marked this pull request as ready for review August 9, 2022 16:11
@LexLuthr LexLuthr requested a review from a team as a code owner August 9, 2022 16:11
@LexLuthr
Copy link
Contributor Author

can we have a test?

Jen, a real world test or an itest? Because itest is already there.

Copy link
Contributor

@geoff-vball geoff-vball left a comment

Choose a reason for hiding this comment

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

This is looking great!! Just a few comments. Let me know if you have any questions :)

storage/wdpost/wdpost_run_faults.go Outdated Show resolved Hide resolved
node/impl/storminer.go Show resolved Hide resolved
cmd/lotus-miner/proving.go Outdated Show resolved Hide resolved
cmd/lotus-miner/proving.go Show resolved Hide resolved
itests/wdpost_config_test.go Outdated Show resolved Hide resolved
itests/wdpost_config_test.go Outdated Show resolved Hide resolved
@LexLuthr LexLuthr requested a review from geoff-vball August 26, 2022 15:19
cmd/lotus-miner/proving.go Outdated Show resolved Hide resolved
cmd/lotus-miner/proving.go Outdated Show resolved Hide resolved
node/impl/storminer.go Outdated Show resolved Hide resolved
storage/wdpost/wdpost_run_faults.go Outdated Show resolved Hide resolved
storage/wdpost/wdpost_run_faults.go Outdated Show resolved Hide resolved
@LexLuthr LexLuthr requested a review from geoff-vball August 29, 2022 08:50
Copy link
Contributor

@geoff-vball geoff-vball left a comment

Choose a reason for hiding this comment

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

Thanks so much for all the quick fixes! Just a few more small changes needed and we can get this merged!

storage/wdpost/wdpost_run_faults.go Outdated Show resolved Hide resolved
storage/wdpost/wdpost_run_faults.go Outdated Show resolved Hide resolved
@LexLuthr LexLuthr requested a review from geoff-vball August 29, 2022 16:04
@geoff-vball
Copy link
Contributor

Just to update the status of this ticket. @Reiers did some testing, found some small issues. @LexLuthr has fixed them, but we're waiting for @Reiers to come back from OOO to retest.

@Reiers
Copy link

Reiers commented Sep 5, 2022

Im running the latest and message still gets sent, going through the check list:

  • lotus-miner proving recover-faults <sectors> still hangs, no receipt, need to be killed. (see example)
  • I need to fault more sectors to check if I can do more than one. ( I will run on the branch, and break some more tonight )
  • lotus-miner proving recover-faults <faulty sectors> is correct in -h
~$ lotus-miner proving recover-faults -h
NAME:
   lotus-miner proving recover-faults - Manually recovers faulty sectors on chain

USAGE:
   lotus-miner proving recover-faults [command options] <faulty sectors>

OPTIONS:
   --confidence value  number of block confirmations to wait for (default: 5)

Example:

~$ lotus-miner proving recover-faults 22861
^C^Z
[4]+  Stopped                 lotus-miner proving recover-faults 22861

@LexLuthr
Copy link
Contributor Author

LexLuthr commented Sep 5, 2022

@Reiers How long does the command stays stuck if you don't kill it?

@geoff-vball
Copy link
Contributor

@Reiers I did some quick testing and it looks like closing the channel (current version) works for me. Can you give it another try when you get a chance?

@Reiers
Copy link

Reiers commented Sep 5, 2022

Sure! I will test first thing tmr.

@Reiers
Copy link

Reiers commented Sep 6, 2022

Closing the channel did the trick - no issues.

@LexLuthr LexLuthr merged commit 67d4f90 into filecoin-project:master Sep 6, 2022
@LexLuthr LexLuthr deleted the feat/manualFaultRecovery branch September 6, 2022 15:47
magik6k added a commit that referenced this pull request Sep 8, 2022
magik6k added a commit that referenced this pull request Sep 8, 2022
ffi: Revert accidental filecoin-ffi downgrade from #9144
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.

Expose mechanism to DeclareFaultsRecovered manually
3 participants