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

enh: initial catchup performance #290

Merged
merged 5 commits into from Sep 20, 2023
Merged

enh: initial catchup performance #290

merged 5 commits into from Sep 20, 2023

Conversation

boudra
Copy link
Member

@boudra boudra commented Sep 19, 2023

This aims to fix the current blocking issue with deployments by reducing the time it takes to do an initial catch up without making too many changes.

Note: There's a temporary change to auto deploy this branch to staging to test deployments, remove before merging.

Depends on boudra/chainsauce#2 boudra/chainsauce#1

IPFS

Some IPFS files do not seem to exist anymore, I can see them on the Pinata dashboard but the IPFS server never returns a response, this happens even when using different gateways, example: https://ipfs-grants-stack.gitcoin.co/ipfs/bafkreig47gxcq5j2mkligw2odnitqqhxfhs34mz5r6yry4jtfgqr54f6yq

This holds the deployments due to high timeouts in the IPFS fetch and finally crashes the indexer when it fails. We could treat it as a warning as the hash comes from the user.

RPC (Chainauce)

Chainsauce's RPC fetching depends on node-fetch and this issue was affecting us: node-fetch/node-fetch#1735

The retry logic is also a bit aggressive and it could overwhelm smaller servers like Fantom or PGN testnet.

Storage access by ID (Chainsauce)

On each vote we check whether the contributor has already donated or not, we do this by looking up the contributor address, this can get expensive as the search is linear and the number of contributors can be very high, this also happens inside the tightest loop of the indexing process so it affects the indexing time considerably, Chainsauce now implements and index to cover cases when we want to find a record by ID.

Memory leak (Chainsauce)

Fixed a bug where Chainsauce kept all the JSON files in memory, they're now be garbage collected as intended when written to the disk. The indexer should use a lot less memory now, looks like production is at 3GB and staging at 1GB running the same chains.

@boudra boudra marked this pull request as draft September 19, 2023 12:50
@boudra boudra changed the title Fix deployments enh: initial catchup performance Sep 20, 2023
@boudra boudra marked this pull request as ready for review September 20, 2023 09:22
.github/workflows/deploy-branch.yml Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
@bard
Copy link
Contributor

bard commented Sep 20, 2023

Snapshot testing against main is passing.

src/index.ts Outdated Show resolved Hide resolved
@bard
Copy link
Contributor

bard commented Sep 20, 2023

@boudra this node-fetch/node-fetch#1735 says it's only Node 19+ but we're using Node 18. Is the issue incorrect? It would be good to know for sure that those ECONNRESET are addressed

@boudra
Copy link
Member Author

boudra commented Sep 20, 2023

@boudra this node-fetch/node-fetch#1735 says it's only Node 19+ but we're using Node 18. Is the issue incorrect? It would be good to know for sure that those ECONNRESET are addressed

I am definitely not seeing them as frequently anymore, I've noticed some from Fantom but the new retry logic just retries that single request instead of splitting a range into two requests, so this will happen less: boudra/chainsauce#1

@boudra
Copy link
Member Author

boudra commented Sep 20, 2023

We need to review & merge these ones first, so I can update the package hash to the latest main branch boudra/chainsauce#2 boudra/chainsauce#1

package.json Outdated Show resolved Hide resolved
@boudra boudra requested a review from bard September 20, 2023 15:13
@boudra boudra merged commit 4a4244e into main Sep 20, 2023
2 checks passed
@boudra boudra deleted the fix-deployments branch September 20, 2023 15:13
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