background overlay: move doing transaction validation in the background before forwarding to tx queue, fast failing invalid transactions

[anupsdf]

No description provided.

[marta-lokhova]

Note that with the switch to BucketList snapshots, we're losing LedgerTxn's internal entry cache, which might be problematic for performance.

[MonsieurNicolas]

Note that with the switch to BucketList snapshots, we're losing LedgerTxn's internal entry cache, which might be problematic for performance.

it sounds like we need a proper performance evaluation to ensure that we don't introduce performance regressions both when closing ledgers (catchup) and when flooding.

[bboston7]

Here are 3 options I see, in order of increasing amounts of backgrounding:

1. Background the signature checks only. With this option, we would check transaction signatures in the background upon receiving them over the network. If signature verification succeeds, we continue with the current Herder::recvTransaction flow in the main thread. Otherwise, we drop the transaction. Then, when core validates the transaction it will achieve a cache hit in the signature cache, thus reducing the total work the main thread has to do. The benefits of this option is that it is small and simplest to get the threading right, while also tackling one of the most expensive parts of transaction validation.
2. Background TransactionFrame::checkValid. With this option, we would run checkValid on transactions upon receiving them over the network. If they pass, they'll be passed on to the transaction queue in the main thread for the remaining checks and final adding to the queue. The benefit of this option is that it moves even more to the background, with the drawback of requiring even more work to achieve thread safety (for example, checkValid must be refactored to not take an Application).
3. Background TransactionQueue::canAdd. This option backgrounds all of the checks run when adding a transaction to the queue. The benefit of this option is that it includes some structural checks that (1) and (2) don't, and therefore it preserves the same order of checks that stellar-core currently performs. The drawback is that it requires even more care to achieve safe multithreading, and I suspect that these structural checks do not make up a significant portion of time taken to add a transaction to the queue. This may be beyond the point of diminishing returns for backgrounding transaction validation.

Note that these options only apply to transactions received over the network. There will be no backgrounding of validation for transactions received over the HTTP endpoint. That will preserve the current order of checks (and therefore error codes) for locally submitted transactions.

I plan to use Tracy to figure out what the most expensive portions of transaction validation are to help answer which of these options we should take, but I also wanted to put these out there to gather feedback from anyone who has an opinion on these three choices (or any others that I may have missed).

[marta-lokhova]

Thanks for putting together the options! I think we can carve out a change that's not too complicated in terms of implementation, but yields a good perf gain. Thinking about this more, a few questions:

• I think option 3 requires a significant amount of work, since this means we'd need to add synchronization to the whole transactions queue class. What do we get in terms of perf gains? For the error checking, we should be able to re-arrange things to avoid unnecessary synchronization.
• checkValid should hopefully be fairly simple to make thread-safe given that we use snapshots now. Also, Cleanup apply and validation flows #4471 refactors checkValid to take AppConnector, which forces us to either run it from the main thread, or implement thread-safe methods. That being said, what benefits do we gain from going the checkValid route vs just signature verification?
• Another idea (a bit more involved, but could yield really good benefits): if we're able to do checkValid in the background, what benefits would be gain from flooding valid transactions right away from the background? This would remove the main thread head-of-line blocking issue that txs face right now. Note that we'd need to do something about the flooding schedule: that is, currently we don't eagerly flood right away, but flood in batches on a schedule (via FLOOD_OP_RATE_PER_LEDGER and FLOOD_TX_PERIOD_MS configs ). The periodic flooding is done from the main thread right now. Implementation-wise, this is closer to option 3, since we'd need to synchronize access to the tx queue, but I think this may yield a pretty dramatic improvement to tx flooding.
• I agree that some experimentation and understanding of end-to-end latency of transaction processing would be really useful to understand the path forward here.

[marta-lokhova]

Thinking more about synchronization at the transaction queue layer: a simpler approach might be removing overlay dependency on the transaction queue completely. This way, overlay would maintain its own pool of transactions to flood, removing the need to share state with main. I believe that would be a step in the right direction, as it would further isolate the "relay" component of core.

[marta-lokhova]

Done in #4699