core: move TxPool reorg and events to background goroutine

[fjl]

This change moves internal queue re-shuffling work in TxPool to a background
goroutine, TxPool.runReorg. Requests to execute runReorg are accumulated
by the new scheduleReorgLoop. The new loop also accumulates transaction events.

The motivation for this change is making sends to txFeed synchronous.

Fixes #19192

[fjl]

This isn't quite done yet. I still need to figure out how to remove the go txFeed.Send(...) in TxPool.add.

[fjl]

The 'aggregation' of promoteExecutables calls probably won't work in practice with this change alone because runReorg and add compete for the central lock.

Consider AddRemote: we take the lock, validate the transaction, move the TX into pending or queue and drop the lock. Then we kick off a background reorg, which takes the lock again, moves TXs around, drops the lock and finally sends events. Since the next call to AddRemote will have to wait for the lock held by reorg before adding its TX, concurrency doesn't improve much. Things are slightly better than before this change when it comes to events though. If a downstream consumer of txFeed is blocked for a while, reorg calls will aggregate instead of launching many goroutines to deliver events.

To really improve high load behavior, we'd need a way to quickly verify the TX without the big lock, submit the request to add it and move on. The reorg stuff could then take batches of transactions periodically and integrate them all at once, keeping the central lock free for readers to take most of the time.

[fjl]

An idea: if we split up the lock so there is one for the queue and one for pending we could just always insert TXs into the queue and then always promote them in the background.

[holiman]

The previous code only caled promoteExecutables if len(dirty) > 0 (in addTxsLocked). Was there a reason to not carry that over?

[fjl]

The request is definitely needed to trigger sending of events even if
there isn't anything to promote. Furthermore, promoteExecutables is pretty
smart about avoiding work.

[fjl]

@holiman PTAL

[holiman]

This PR (specifically) instance=Geth/v1.9.0-unstable-87409422-20190614/linux-amd64/go1.12.6 is now running on

bootnode-aws-us-east-1-001
bootnode-aws-ap-southeast-1-001
bootnode-azure-australiaeast-001
bootnode-azure-westus-001

[holiman]

Here are the charts after a few days running. The machines are LES servers, and have around 490 peers each, the upper chart shows the one running this PR, the lower ones some old version which is fairly recent (but does not have this PR (nor some other changes in the LES area)

The blip on friday evening is when they were restarted with this PR.

The non-updated machines have a lifetime of around 24-36 hours before they crash, with stack traces that point to extreme goroutine overload. So yeah, this PR looks good to me from that perspective!

[holiman]

Seeing quite a lot of these in the logs;

ERROR[06-17|06:25:47.909] Demoting invalidated transaction         hash=d51870…cd6d3d
ERROR[06-17|06:25:47.946] Demoting invalidated transaction         hash=560c07…7c2506
ERROR[06-17|06:25:47.965] Demoting invalidated transaction         hash=d74ba3…9629c2
ERROR[06-17|06:25:47.991] Demoting invalidated transaction         hash=75b0b8…950046

If it is truly ERROR, we need to look into it. If it's expected, it shouldn't be ERROR level

[holiman]

Error messages comes from demoteUnexecutables, which is called from reset.

	// Inject any transactions discarded due to reorgs
	log.Debug("Reinjecting stale transactions", "count", len(reinject))
	senderCacher.recover(pool.signer, reinject)
	pool.addTxsLocked(reinject, false)

	// validate the pool of pending transactions, this will remove
	// any transactions that have been included in the block or
	// have been invalidated because of another transaction (e.g.
	// higher gas price)
	pool.demoteUnexecutables()

The reset tries to take transactions that were in a block that became reorged and did not make it into the new block, and shove them back into the pending. However, the addTxsLocked in this PR is async, and they don't immediately wind up in pending.

Afterwards, it tries to demote unexecutables, and finds that the subsequent transactions are still there, and it becomes surprised since the reinjceted ones are now not there. And the effect is that the subsequent ones get shoved out from pending and into queued.

[holiman]

This PR against master. The four machines running this PR are all around 2.5K goroutines, glued to the bottom of the chart. The ones running master are going haywire.

[holiman]

Future bootnodes now updated to run this PR 0dbe8bd against master