Optimize Orderbook loading to use sql rows stream directly

[bartekn]

Currently GetAllOffers is used to load all offers from a database into in-memory order book graph. Currently, there are around 33k offers in the public network however, if this number increases in the future this function may require more memory. Instead, we should stream offers from a DB using sql.Rows. We already use this method in several places in Horizon so check them for reference.

[ire-and-curses]

Offers have gone up 10x; agree this is important.

[sreuland]

Hello @bartekn , after some review with @tamirms , it looked like sql.Rows.Next() may already be getting invoked. I stepped through offers.go:GetAllOffers to confirm, looks like it does result in call stack that ends up invoking the same sql.Rows.Next() from other sqlx wrappers, the underlying lib/pg driver enables the streaming behavior on that by buffering frames from server for the result set rather than all up front, keeping mem usage down, successive calls to sql.Rows.Next() trigger accepting new frames from server. So, this should already be performing streaming behavior as requested.

[bartekn]

@sreuland it's possible that rows are streamed in sqlx. The problem is all the stream rows are then added to a result []Offer slice in GetAllOffers. We already have a method in db/support.Sesson that can allow us to do it: Query. It returns *sqlx.Rows we can use to iterate on rows one by one. I was thinking we use it already in certain places in Horizon but we don't (maybe these were removed) but we should start using Query here. Let me know if you have further questions.

[sreuland]

Ok, I may have misread on expectation, is the request to address the receiving []Offer slice growing too large after the rows have been scanned in? Just to confirm the call stack currently results in the same iteration of rows:

iteration of results with sql.Rows.Next() is encapsulated in sqlx.scanAll(), are you requesting to do the iteration on sql.Rows.Next() directly in GetAllOffers instead, not sure of net benefit? Let's try call tomorrow, will get confirmation, thanks!

[bartekn]

I'm suggesting to change GetAllOffers to return sqlx.Rows and them to the graph one by one. This way we won't need to keep all offers in memory at any point in time.

[sreuland]

Ok, got it, the loading of the orderboook graph should be refactored to load direct from sql.Rows iteration and remove the intermediate step of loading the sql.Rows to a slice first. Also, during investigation on this with @tamirms, he mentioned it would likely be worthwhile to do the same refactor for GetAllLiquidityPools as it follows the same graph loading pattern from sql rows. Let me know if this is not accurate, otherwise, I should be able to proceed, thanks!