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

fixes double lookups when block does not exist #739

Merged
merged 4 commits into from
Mar 15, 2024
Merged

Conversation

gmega
Copy link
Member

@gmega gmega commented Mar 12, 2024

fixes #699

CodexNodeRef.retrieve always tried to refetch manifests for which fetchManifest fails, even if these failures were not due to decoding errors (which would indicate that the CID was pointing to a block which was not a manifest) but timeouts (which would indicate that the CID does not exist, and we should just return with a failure).

This means that, if the CID does not exist, we had to wait for twice the timeout (currently 20 minutes) before getting an error. This PR fixes that, so that we lookup only once and the actual timeout is respected.

Copy link
Contributor

@dryajov dryajov left a comment

Choose a reason for hiding this comment

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

We should figure something out with retries and such, but this looks good for now 👍

Copy link
Contributor

@benbierens benbierens left a comment

Choose a reason for hiding this comment

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

Nice improvement!

@gmega gmega added this pull request to the merge queue Mar 15, 2024
Merged via the queue into master with commit d33804f Mar 15, 2024
10 checks passed
@gmega gmega deleted the fix/double-lookups branch March 15, 2024 22:45
@AuHau
Copy link
Member

AuHau commented Mar 19, 2024

While nice fix, I don't think it "solves" the issue I originally created. Having 10 minutes ongoing API call is IMHO still "hanging"... I mean beater 10 minutes then 20, but still...

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.

POST /storage/request hangs, when manifest can't be retrieved
4 participants