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

Retry with exponential backoff for stream opening #382

Merged
merged 5 commits into from Sep 2, 2020

Conversation

aarshkshah1992
Copy link
Collaborator

@aarshkshah1992 aarshkshah1992 commented Aug 27, 2020

Closes filecoin-project/lotus#3416.

@hannahhoward Should we do this for the retrieval market as well ?

Copy link
Collaborator

@hannahhoward hannahhoward left a comment

Choose a reason for hiding this comment

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

LGTM. Yes, let's do this for retrieval as well.

@hannahhoward
Copy link
Collaborator

make sure to fix lint errors before merge

Copy link
Collaborator

@hannahhoward hannahhoward left a comment

Choose a reason for hiding this comment

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

Hey @aarshkshah1992 -- I'm getting a freeze in the tests when I rebased on master after merging your other PR (the restart testing). Can you rebase on master and investigate?

@aarshkshah1992
Copy link
Collaborator Author

@hannahhoward On it.

@aarshkshah1992
Copy link
Collaborator Author

aarshkshah1992 commented Sep 1, 2020

@hannahhoward

  1. There was a very deep bug in the test wherein because of using randomly generated temporary file paths, we weren't being to find the piece file across restarts. Because of this, the provider failed when trying to do a HandoffDeal on the restart. Have fixed that bug now and the tests look good on repeated runs.

  2. Have added the backoff-retry logic for the retrieval markets as well.

@hannahhoward
Copy link
Collaborator

@aarshkshah1992 this great but it looks like you still have lint and test failures. At least one of the test failures looks non-flaky and needs a change. (TestProviderStop)

@aarshkshah1992
Copy link
Collaborator Author

aarshkshah1992 commented Sep 2, 2020

@hannahhoward Fixed the linting issue and test. Please take a look. The other failures are flakies for which I've created an issue.

@hannahhoward hannahhoward merged commit 90b72dd into master Sep 2, 2020
@aarshkshah1992 aarshkshah1992 deleted the fix/network-retries branch September 3, 2020 04:10
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.

Recover from dialing errors
2 participants