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

Duplicate CL positions query #3168

Closed
ValarDragon opened this issue Apr 26, 2024 · 1 comment
Closed

Duplicate CL positions query #3168

ValarDragon opened this issue Apr 26, 2024 · 1 comment

Comments

@ValarDragon
Copy link
Member

ValarDragon commented Apr 26, 2024

Currently we do two queries for CL positions, and I'm confused why we do two sets.

First we do:

The latter set of queries don't need to happen imo. I can't tell if the second set of queries is what is populating the cards, I think so but could very well be wrong due to my imprecise checking method.

@jonator
Copy link
Member

jonator commented Apr 26, 2024

Oh I see. These are in the tRPC procedures running locally in browser.

Yes, the latter queries (used in getPositionDetails procedure) are fetching details for positions that are visible. If the user has >3 positions and clicks "see more" button then it would wait to run the latter queries on those positions that become visible.

Yes. This could be cleaned up, perhaps by passing the per-position data needed by getPositionDetails as input to avoid the position_by_id query. Maybe we can make things flexible by making that input optional and it would still allow callers only need the position ID as requirement to call the procedure.

The performance of these queries is highly dependent on the locality of the underlying services used in the query relative to the edge function or user (which could be considered the same, theoretically since edge functions are supposed to be close to user). Often we send requests concurrently, making the slowest query the performance bottleneck. But other times, the very slow procedures need to send queries serially because of interdependence of the queries. In that case things can be very slow like with getUserPools. Currently I'm thinking adding this processing to SQS, which is pinned close to the node and/or could ingest the needed data. Alternatively, until we get more regions I'm thinking perhaps of pinning edge functions to regions that are closer to the data dependencies. @niccoloraspa 's effort to make query nodes multi region will be a big help, as that's the only non-multi-region data dep we have now.

This is a con of our migration to tRPC. It greatly simplifies client code by bundling multiple async opts into one, but then the above pitfalls are revealed. Before, every async op (query) would update the view incrementally as the dependent data was loading. Notice in old assets page, the numbers at the top of the page gradually increase as the various queries for your assets are responding. So our old arch "felt" faster since the fast queries would update the view immediately, but the client was more complex and we sometimes had weird race condition bugs or debounce/thrashing issues with the view getting re-rendered so often.

Thoughts?

@linear linear bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 4, 2024
@jonator jonator closed this as completed Sep 4, 2024
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

No branches or pull requests

2 participants