This is on Linux only. Who knew? Thanks for Jt for finding this bug.
We had to go ahead and get rid of the Util::AsyncOnObject() call in
putBlock and lieu of a signal/slot connection (putBlock connected to
protected on_putBlock).
This fixes the race condition observed on Linux only whereby blocks
would be done downloading but the download task would get deleted before
the blocks could get processed by the Controller.
By making all of the events the Controller listens for arrive via
signals now, we avoid any out-of-order issues since signals themselves
always get delivered in order (whereas it appears timers get processed
AFTER the signals but only on Linux! On windows and macos they do not!).
management of mempool data
rehash the fixed sized maps/sets to their fixed size to make load factor
as close to 1.0 as possible (thus not wasting space).
We enforce re-use of the same underlying memory for all scriptHashes as
we update the mempool. This should significantly save memory in
pathological/large mempool situations where many of the tx's involve
the same address.
We also compactified the mempool data structure a bit, removed unused
data members (depends, spentBy), and decided to calculate fee ourselves
by doing ins - outs. (This avoids potential problems we may have seen
where doubles cause off-by-one errors for sats).
Also cleaned up Controller::process a little bit to have a well-defined
state when "waiting for chain info".
Made the txos be a vector, rather than a slow-ish/bloated map. This
should be much faster.
Also various other small tune-ups.
Added "Broadcast tx.." message to log on broadcast.
More to come!
- Made the hashXTxs mapped_type be a std::vector<TxRef> rather than a
std::set<TxRef>. We manually maintain the uniqueness and sort invariant
as we process the mempool. Vectors are much more compact memory-wise
and also have better insert characteristics, so we prefer them here.
Also, we never check for set membership, so the O log N lookup of the
set is not warranted.
We do however potentially build very large hashX -> TxRef arrays so a
std::vector is much better.
After this fix, we need to see what else in this data structure is
wasteful. I may get rid of the "txos" std::map<IONum, TXOInfo> in favor of a
simple and direct std::vector<TXOInfo>
- The crash bug was due to misuse of
robin_hood::unordered_flat_map::erase (it's still kind of bizarre that
it would crash the way it did, but I suppose the next iterator was
invalidated somehow). We instead grab the next returned by .erase() and
now it works ok.
- We redid the ownership model of SubsMgr. Rather than having
Controller own it and pass it all the way down to the Server instances,
we instead have Storage create & own it. This is needed because in the
future Storage may need to invalidate cached sub statuses as blocks are
undone in undoLatestBlock, and/or it may need to atomically add
notifications as blocks are added. So it makes sense for Storage to own
the SubsMgr (as it also owns the Mempool, which has similar ephemeral
qualities).
Not sure why it was crashing in debug mode in ~CtlTask but I think it
was due to a compiler bug using __FUNCTION__ + Debug(printf_format). We
switched it up and it no longer crashes.
The rule is: debug mode prints every time it changes to debug logger.
Normal logger prints at most once every 30 seconds, and only if the
mempool is non-empty and if the size has changed, and if "up-to-date"
(in Controller).
The logic is a bit convoluted, but at least it doesn't print too often.
We wrapped the QCache class which is a very light weight, cost-based LRU
cache. It wasn't thread safe so we wrapped all its methods in
thread-safe versions.
Now we have a very accurate bound on memory usage for caching. We may
want to replace the other height2txhash cache with this new class as
well since this new cost cache seems to be much more efficient.
Just paranoia/defensive programming.
Also added a check such that if the GetChainInfoTask fires multiple
times before a response comes in from bitcoind (in case bitcoind is out
to lunch for a few seconds), only the most recent request is
responded-to.
Solved a problem that bugged me since the beginning: how to make latency
for the server as low as possioble and maximize CPU core usage.
We use a ThreadPool which gets invoked via generic_do_async() in
Servers.cpp for the more expensive rpc_* calls. All the ones that may
hit the db and take a while, or may churn on the CPU (such as the
merkle-related ones) immediately go async and schedule their work on the
threadpool.
The rule will be only the most trivial rpc methods generate results
immediately, the rest schedule work via the threadpool.
This will help us scale and leverage as many cores as possible. It also
allows us to have many many clients "living" in the server's thread
(thus reducing lock usage).
I'm excited about this design.
This allows for an app-global threadpool to be used for work that may
take a little while to complete. The intended code that will use this
is the rpc server which will submit work that takes some time to
complete to the ThreadPool, so that the server's thread doesn't block
for very long while servicing client requests.
This ensures that the server remains responsive even if 1 or 2 clients
are issuing costly requests.
We plan on using this for get_history, listunspent, and the
merkle-related functions, to name a few.
- blockchain.transaction.get is now fully implemented
- blockchain.transaction.broadcast is now fully implemented
Note we emulate electrumx's quirky/inconsistent behavior as much as
possible within reason until we can determine we can "do the right
thing" and it won't break existing clients.
There was a race condition before if the controller thread was very busy
it would not have time to reconnect the "wait for auth" signal and as
such would end up in a state where it kept spamming "waiting for
bitcoind" to the log incessantly.
This has been handled by keeping the gotFirstGoodConnection signal
always connected and only reacting to it if the "lostConn" flag is
latched (latched by allConnectionsLost signal).
The gotFirstGoodConnection signal is only emitted when the first good
connection is made anyway so it's not spammed. It was not ideal that we
didn't have it always connected.
This fix hopefully fixes the race condition we saw when synching mainnet
on my slow windows box.
We deleted the non-working merge operator for scripthash_unspent.. and
we do a read-modify-write for each new utxo/spend per scripthash.
It's very slow though.
I may have to defer updating this table until the very end from the utxo
set or something. This is unacceptably bad...
