-
Notifications
You must be signed in to change notification settings - Fork 72
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: retry offline deal after commp errors #899
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good so far, just a couple of suggestions
storagemarket/deal_commp.go
Outdated
retry: types.DealRetryManual, | ||
error: fmt.Errorf("commP mismatch, expected=%s, actual=%s", clientPieceCid, pieceCid), | ||
} | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} else { |
storagemarket/deal_commp.go
Outdated
@@ -56,6 +65,13 @@ func (p *Provider) generatePieceCommitment(filepath string, pieceSize abi.Padded | |||
var err error | |||
pi, err = GenerateCommP(filepath) | |||
if err != nil { | |||
// Allow auto retry to cover cases where IO copy fails due to lack of space | |||
if strings.Contains(err.Error(), "failed to write to CommP writer") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of doing string matching on the error string, let's use a custom error.
See for example ErrDealHandlerNotFound
Line 178 in 015b654
if errors.Is(err, storagemarket.ErrDealHandlerNotFound) { |
Could you please also add some cases to TestDealAutoRestartAfterAutoRecoverableErrors and TestDealRestartAfterManualRecoverableErrors (or create new tests if that's simpler) to verify that the changes you've made work correctly. The existing tests are in storagemarket/provider_test.go |
6d31c40
to
799c08c
Compare
799c08c
to
69bb812
Compare
fixes #895