Optimize history tables for query by id performance(claimable balance, accounts, liquidity pools and assets)

[sreuland]

What version are you using?

Horizon 2.18.1

What did you do?

https://horizon.stellar.org/claimable_balances/0000000028b32c721976af53fa0419eba129f3da70168f1e39822a25886375abad9599da/transactions?limit=1&order=asc

What did you expect to see?

list of transactions related to entity returned in reasonably fast response time

list of tx's by entity covers a few types that can be vulnerable to this query path behavior, 'claimable balance', 'accounts', 'liquidity pool', 'assets', want to include optimization for each of these as they likely have same root cause in query performance.

What did you see instead?

503, timeout responses.

[sreuland]

this issue was identified by @jcx120 and triaged in slack thread, @sydneynotthecity investigated the underlying sql query for the endpoint and mentioned in separate thread that successful IS NULL on tx table in where clause is highly likely to be cause, need to identify if that check on history_transactions.successful is necessary and if not remove, and verify performance improvement.

[sydneynotthecity]

Another item I would propose: changing the LEFT JOIN clause to an INNER JOIN on history_ledgers table. My assumption is that the LEFT JOIN ensures if a transaction is written to the history_transactions table prior to history_ledgers closing the current ledger, the transaction would still appear. Is this a frequent edge case that happens for this route? Would we be able to accept the risk that a transaction submitted in <5 seconds from current time might not be in the return results?

[sydneynotthecity]

Update: optimizing the query does not provide measurable benefit to query runtimes; I recommend we leave the query unchanged.

If we update the WHERE successful is NULL condition and accept the risk of dropping records by removing the LEFT JOIN, we only reduce the query runtime by 3000 ms (the query on average takes 184,000 ms to execute) which is a reduction of ~1.5%

@sreuland do you know if we can add another index to this table? The table only has the following indices:

"index_history_transaction_claimable_balances_on_ids" UNIQUE, btree (history_transaction_id, history_claimable_balance_id)
"index_history_transaction_claimable_balances_on_transaction_id" btree (history_transaction_id)

Since there is no index with history_claimable_balance_id only, the query you sent is performing a full table scan. My hypothesis is adding this additional index would greatly reduce query runtime (at the cost of storing the index)

[sreuland]

@sydneynotthecity , I'll try new recommended index on staging db, it has a full copy of the pubnet data set on it's postgres instance, should be able get decent estimate of the net effect to this query, thanks!

[sreuland]

I think we should expand the scope of this ticket to cover operations by cb also, as those are triggering slow timeouts, alerts also.

[sydneynotthecity]

@sreuland the history_operation_claimable_balances has the same indices as the history_transaction_claimable_balances table:

"index_history_operation_claimable_balances_on_ids" UNIQUE, btree (history_operation_id, history_claimable_balance_id) 
"index_history_operation_claimable_balances_on_operation_id" btree (history_operation_id)

I bet if we add a third index with history_claimable_balance_id only that the query cost would reduce for operations as well.

We're have to ensure this additional index wouldn't impact routes that search on operation/transaction only if that's a common access pattern for this API.

[sreuland]

@sydneynotthecity @jcx120 , i tested on staging db the suggestion of new indexes from Syd, the staging db has full pubnet dataset, so it's comparable for measurement, the fix works great for ops and txs, the response time is now sub-second, can contrast the two here:

staging with indexing
prod without indexing

thanks Syd!

[sydneynotthecity]

@sreuland do you know if any other Horizon route uses those two tables? Want to confirm that we're not impacting other existing traffic by adding the new indices to the tables. My assumption is this is the only route but want to be certain.

[jcx120]

Also in addition, would be curious to see if adding these indices significantly impacts DB storage footprint.

[sreuland]

@sydneynotthecity , it looks like usage of history_transaction_claimable_balances and history_transaction_claimable_balances limited to queries for just these endpoints here /claimamable_balances/{cb_id}/[operations|transactions].

@jcx120 , I pulled the size of these new indexes, each took about 5.7GB of space in the DB, so a combined total of 11GB, buty that is just based on todays ledger size, that would grow as more cb entries are created.

[2opremio]

@sreuland l, as @sydneynotthecity suggested at #4396 we should consider the same indices for other objects (accounts, liquidity pools and assets)

[sreuland]

@2opremio , @jcx120
Per suggestion, I expanded the scope of acceptance criteria to include review and optimization of query-by-tx for a wider range of entity types beyond just cb's, such as accounts, liquidity pools and assets.