itests: retry deal when control addr is out of funds #7454
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nonsense
force-pushed
the
nonsense/retry-in-publish-deal
branch
5 times, most recently
from
3013fed
to
42017b9
Compare
dirkmc
reviewed
Oct 7, 2021
| propcid := *deal | ||
|
|
||
| go func() { | ||
| time.Sleep(20 * time.Second) |
There was a problem hiding this comment.
Can we subscribe to deal events, and watch for publish, rather than using a sleep here? (or maybe subscribe to deal events and use a shorter sleep)
There was a problem hiding this comment.
I will certainly use a shorter sleep. We can monitor for Publish, but you can't really tell if the Publish handler did something or not, and this is really what we are interested in.
Codecov Report
@@ Coverage Diff @@
## master #7454 +/- ##
=======================================
Coverage 39.80% 39.81%
=======================================
Files 631 631
Lines 66851 66894 +43
=======================================
+ Hits 26611 26631 +20
- Misses 35618 35641 +23
Partials 4622 4622
Continue to review full report at Codecov.
|
nonsense
force-pushed
the
nonsense/retry-in-publish-deal
branch
from
f4f5c12
to
4938e2f
Compare
nonsense
force-pushed
the
nonsense/retry-in-publish-deal
branch
from
4938e2f
to
a19dbb8
Compare
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Should fix: #7102
Related: filecoin-project/go-fil-markets#638
I am trying to come up with a solution that doesn't involve adding another 1-2 states to
go-fil-transferand another 1-2 events and adding more state transitions.I think a deal being stuck in the
Publishstate is the natural thing to happen in case we fail to transition to thePublishingstate, i.e. we fail to initiate the PSD message.Having discussed within the #ignite team, we decided against automatically trying to retry PSD messages. With this PR, a PSD message would be retried only if the node is bounced, as a
Restartevent is sent to deals that are not in a terminal state.TODO:
Publishstate, unless we callMarketRetryPublishDealgo-fil-marketsref when related PR is merged so that we don't refer to a commit that's not onmaster