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

Support removeProxies in the indexer #285

Merged
merged 3 commits into from
Aug 16, 2023
Merged

Conversation

Tbaut
Copy link
Collaborator

@Tbaut Tbaut commented Aug 10, 2023

closes #283

This is non trivial to properly test, due to the way the indexer works. There are 3 types of scenarios:

  1. it's a new event. Meaning that you call proxy.removeProxies now, on an old pure, that is already persisted in the DB
  2. it's a recent event compare to the proxy creation, but we re-index the chain. Meaning that the proxy.removeProxies will be in a batch with potentially many other proxies creation/deletion
  3. you create a proxy or a pure, then shortly after a proxy.removeProxies, and we re-index the chain (like on 2). Then the new proxy or the pure will never even touch the DB. The removal will happen in a batch array.

I've carefully tested all these scenarios. There is no easy way to test 2 and 3 because it requires a re-indexing, which happens every month, when we have changes in the indexer.

In any case the deployment of the test UI is now pointing to the new (v2) of the indexer that supports it. @LernaJ you should not see the pure proxies any more (the multisig are here still though)

Copy link
Member

@asnaith asnaith left a comment

Choose a reason for hiding this comment

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

The New Event scenario is working well. The pure removal went smoothly, and the UI updated as expected without needing a refresh.

For the Recent Event with Chain Re-Indexing and Concurrent Creation and Removal with Chain Re-Indexing scenarios, I couldn't fully test them due to indexer knowledge / access but I think the testing you have already done is sufficient here. It seems unlikely that critical proxies would be created and removed so rapidly in the real world.

@Tbaut
Copy link
Collaborator Author

Tbaut commented Aug 11, 2023

Thanks. @LernaJ will be able to test the behavior with re-indexing because she did call the proxy.removeProxies in the past.
Also just to be clear, I can manually launch those re-index, so I could test all these scenarios locally.

@Tbaut
Copy link
Collaborator Author

Tbaut commented Aug 11, 2023

FWIW I hibernated the v2 to see if this is what's causing the slowness. As a result, this PR can't be tested any more, and obv shouldn't be merged before restarted.

@Tbaut Tbaut added Status: Do not merge Added to PRs that are not allowed to be merged. and removed Status: Do not merge Added to PRs that are not allowed to be merged. labels Aug 11, 2023
@Tbaut Tbaut merged commit a620637 into main Aug 16, 2023
6 checks passed
@Tbaut Tbaut deleted the tbaut-support-removeProxies branch August 16, 2023 09:14
@Tbaut
Copy link
Collaborator Author

Tbaut commented Aug 16, 2023

The v2 was started again yesterday, and it doesn't seem to have been the cause of the issue. It was rather an incident on Subsquid side.

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.

proxy.removeProxies isn't taken into account by the indexer
2 participants