RIPD-1847 fix select query condition

[oleks-rip]

High Level Overview of Change

perf: improve account_tx SQL query:

The witness server makes heavily use of the account_tx RPC command. Perf
testing showed that the SQL query used by account_tx became unacceptably slow
when the DB was large and there was a marker parameter. The plan for the query
showed only indexed reads. This appears to be an issue with the internal SQLite
optimizer. This patch rewrote the query to use UNION instead of OR and
significantly improves performance. See RXI-896 and RIPD-1847 for more details.

Example

was:

SELECT AccountTransactions.LedgerSeq,AccountTransactions.TxnSeq,
  Status,RawTxn,TxnMeta
  FROM AccountTransactions, Transactions WHERE
  (AccountTransactions.TransID = Transactions.TransID AND
  AccountTransactions.Account = 'rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh' AND
  AccountTransactions.LedgerSeq BETWEEN '415398' AND '415441')
  OR
  (AccountTransactions.TransID = Transactions.TransID AND
  AccountTransactions.Account = 'rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh' AND
  AccountTransactions.LedgerSeq = '415442' AND
  AccountTransactions.TxnSeq <= '152')
  ORDER BY AccountTransactions.LedgerSeq DESC,
  AccountTransactions.TxnSeq DESC
  LIMIT 400;

became:

SELECT AccountTransactions.LedgerSeq,AccountTransactions.TxnSeq,Status,RawTxn,TxnMeta
  FROM AccountTransactions, Transactions WHERE
  (AccountTransactions.TransID = Transactions.TransID AND
  AccountTransactions.Account = 'rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh' AND
  AccountTransactions.LedgerSeq BETWEEN 415398 AND 415441)
UNION
SELECT AccountTransactions.LedgerSeq,AccountTransactions.TxnSeq,Status,RawTxn,TxnMeta
  FROM AccountTransactions, Transactions WHERE
  (AccountTransactions.TransID = Transactions.TransID AND
  AccountTransactions.Account = 'rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh' AND
  AccountTransactions.LedgerSeq = 415442 AND
  AccountTransactions.TxnSeq <= 152)
  ORDER BY AccountTransactions.LedgerSeq DESC,
  AccountTransactions.TxnSeq DESC
  LIMIT 400;

Type of Change

• Bug fix (non-breaking change which fixes an issue)

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.95%. Comparing base (69143d7) to head (9a0c4df).

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #4955   +/-   ##
========================================
  Coverage    76.95%   76.95%           
========================================
  Files         1127     1127           
  Lines       131696   131695    -1     
  Branches     39578    39520   -58     
========================================
+ Hits        101341   101345    +4     
+ Misses       24440    24398   -42     
- Partials      5915     5952   +37     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[intelliot]

During Sidechain performance testing, we (@sophiax851 and team) found that account_tx's response time increased from a few milliseconds to over 3 seconds when its Witness server pulled transactions from the issuing chain using account_tx for both the door_account and the Witness server's transaction submission account. Three specific examples were found.

1. When the Witness server receives a validated ledger from the ledger subscription stream.
2. One of the subsequent calls issued after receiving the response of the initial account_tx which involves marker for stepping through the transactions.
3. A slow response time when getting transactions from a single ledger: 2982941 microseconds.

There were frequent 2-10 sec delays. We produced a stack trace of the method (Node.cpp:1185) that was causing the slowness. The root cause is the slow database read in accountTxPage (backend/detail/impl/Node.cpp line 1146).

The original query was using index during normal cases, which was showing by the execution plan when query manually, however, apparently at run time, sqlite optimizer would decide to do full table scan as explained in here. Olek found that the delay could be eliminated by improving the query to use "UNION" (instead of "OR"). The change enables use of an index, which most likely improves the query complexity from O(N) to O(logN) (with some fixed constant, dependent on the disk IO speed).

Fixing this bottleneck also improved the response time of account_tx in certain situations - in particular, as the difference between ledger_index_min and ledger_index_max increases (regardless of the limit used).

In May 2024, infra providers found that RPC nodes were crashing in lockstep. In particular, while running account_tx/tx, the nodes would try to find offers to AMM, and continue searching for a long time. Eventually the nodes deadlock and crash. It was found that this was the same issue that was found during Sidechain/Witness perf testing. In particular, account_tx requests with some ledger_index range, e.g.:

        "ledger_index_max" : 87890671,
        "ledger_index_min" : 87890662,

and a marker - can cause slowness.

Clio was not affected.

This PR fixes the issue in rippled version 2.2.0.

In the long run, we could revisit the SQL engine choice or its use by rippled. It’s single threaded currently, even with reading, which was causing the rippled to stall when one query is doing full table scan. In the very long run, the solution is to use Clio instead.