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

[ASI-593] Check all gateways if content cant be found in user replica set and IPFS #2024

Merged
merged 13 commits into from
Nov 24, 2021

Conversation

dmanjunath
Copy link
Contributor

Description

Because we added skips in the sync logic recently, it's possible content is still on another node but our node is unable to retrieve it since it doesn't try all other gateways. This leverages an existing code path that searches (with a cache to avoid retrying more than once a day).

Tests

Tested locally to make sure code flow works. Underlying function used has been in production for over a year.

How will this change be monitored?

Loggly logs

@dmanjunath
Copy link
Contributor Author

@dmanjunath dmanjunath marked this pull request as ready for review November 5, 2021 21:10
Copy link
Contributor

@SidSethi SidSethi left a comment

Choose a reason for hiding this comment

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

nice. one concern is that currently findCIDInNetwork will try every registered node, even though the user's replica set has already been tried. could we add an exclude array param and pass in current replicaset?

….com:AudiusProject/audius-protocol into dm-try-all-gateways-saveFileForMultihashToFS
@dmanjunath
Copy link
Contributor Author

@SidSethi updated to de-dupe

const attemptedStateFix = await getIfAttemptedStateFix(filePath)
if (attemptedStateFix) return

// get list of creator nodes
const creatorNodes = await getAllRegisteredCNodes(libs)
if (!creatorNodes.length) return

// Remove excluded nodes from list of creator nodes, no-op if empty list or nothing passed in
const creatorNodesFiltered = creatorNodes.filter(c => excludeList.includes(c.endpoint))
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be !excludeList.includes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah yes you are correct! sorry its been a long day lol

Copy link
Contributor

Choose a reason for hiding this comment

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

hahaha all good dude - would be good to make sure new code path still works

also i re-ran maddog ci since it failed for non-relevant reason

@dmanjunath dmanjunath merged commit 9305711 into master Nov 24, 2021
@dmanjunath dmanjunath deleted the dm-try-all-gateways-saveFileForMultihashToFS branch November 24, 2021 15:32
@SidSethi SidSethi changed the title Check all gateways if content cant be found in user replica set and IPFS [ASI-593] Check all gateways if content cant be found in user replica set and IPFS Dec 2, 2021
@AudiusProject AudiusProject deleted a comment from linear bot Sep 11, 2023
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.

None yet

2 participants