osmosis-frontend icon indicating copy to clipboard operation
osmosis-frontend copied to clipboard

Duplicate CL positions query

Open ValarDragon opened this issue 1 year ago • 1 comments

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

First we do:

  • https://lcd.app.osmosis.zone/osmosis/concentratedliquidity/v1beta1/positions/{addr} -- 190ms
    • queries all CL positions, with a high pagination bound. (Aside: Probably should be requesting multiple instances of smaller pages?)
  • Then 600 ms later, we do many https://lcd.app.osmosis.zone/osmosis/concentratedliquidity/v1beta1/position_by_id?position_id={your position ID}, that returns the exact same info returned before. On my machine each of these queries takes 170ms.

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.

ValarDragon avatar Apr 26 '24 06:04 ValarDragon

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?

jonator avatar Apr 26 '24 13:04 jonator