-
Notifications
You must be signed in to change notification settings - Fork 296
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
multi: Add UtxoCache. #2591
multi: Add UtxoCache. #2591
Conversation
I'm super happy to see this! I'll be tackling this review in stages between other things given it touches consensus critical areas and thus will require a very careful and time-consuming review. I'll update this list to help give an idea of overall progress. Review:
Testing:
|
We'll need to update the minimum recommended specs in the main README.md to 2GB. |
I marked this as a database upgrade even though it technically isn't 100% one because you can't really go back without causing potential issues when you upgrade again. |
Updated the README.md to now recommended a minimum of 2GB of memory. |
In the last push, I also updated the cache flush log message to only print the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicely done, looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As of this review, I've now finished reviewing all of the code. Very nice job overall and the test coverage is solid. I know this was a lot of work, but it will have a major impact not only at the current time, but, perhaps even more importantly, it significantly improves scalability.
I have just a few inline comments regarding the tests and I'll begin testing the scenarios outlined in an earlier comment and provide feedback with the results.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested the following scenarios and, aside from the aforementioned issue, everything is working as expected:
- Sync from new database (HDD and SSD)
- Sync from existing older upgraded database
- Reconciliation after hard killing the process during sync
- Reconciliation after physically removing hardware from the system during sync (disconnect USB drive)
- Double spend attempts on flushed entries
- Double spend attempts on unflushed entries
- Intentional corruption of the underlying database to ensure appropriate errors happen
65894d6
to
5cddcbc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've gone over this several times now and tested it quite thoroughly. Looks good to go!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran this PR through a whole bunch of IBDs and also for a decent amount of normal operation. All looking stable and improvements are very noticeable.
Some benchmarking data from a system running NVMe SSD + Ryzen 2700x, syncing from a local peer.
Without indexes
Code | Network | Time | Notes |
---|---|---|---|
master | mainnet | 61m 1s | |
testnet | 84m 24s | ||
PR - 100MiB cache | mainnet | 44m 26s (-27%) | Min cache hit ratio: 98.65% |
testnet | 67m 44s (-20%) | Min cache hit ratio: 99.85% | |
PR - 2000MiB cache | mainnet | 36m 32s (-40%) | Min cache hit ratio: 100.00%. Max cache size 261.05 MiB (3,139,811 entries) |
testnet | 58m 39s (-31%) | Min cache hit ratio: 100.00%. Max cache size 243.06 MiB (2,025,762 entries) |
With --addrindex
and --txindex
Code | Network | Time | Notes |
---|---|---|---|
master | mainnet | 98m 53s | |
testnet | 128m 38s | ||
PR - 100MiB cache | mainnet | 84m 6s (-15%) | Min cache hit ratio: 98.99%. Noticed during this run that the cache size can exceed the max specified - 100.01MiB vs 100MiB max. |
testnet | 114m 39s (-11%) | Min cache hit ratio: 99.86% |
Thanks for testing, @jholdstock!
This is expected, it can go over the max size by the total size of the outputs of 1 block (at most), which is pretty minimal, since it only checks the size and potentially flushes after connecting each block. |
Mind rebasing this? It was already behind master a bit and there is a minor merge conflict now. |
This splits the utxo packed flags into two separate types, utxoState and utxoFlags. The reasoning is that: - This cleanly separates the purpose of the flags. utxoState defines the in-memory state of a utxo entry, whereas utxoFlags defines additional information for the containing transaction of a utxo entry. - This makes room for an additional state that is required for the utxo cache, namely whether or not a utxo entry is fresh (does not exist as an unspent transaction output in the database).
This adds utxoStateFresh to UtxoEntry to indicate that a txout is fresh, which means that it exists in the utxo cache but does not exist in the underlying database. The utxo cache will use the fresh flag as an optimization to skip writing to the database for outputs that are added and spent in between flushes to the database.
This adds a size method to UtxoEntry, which returns the number of bytes that the entry uses on a 64-bit platform. This will be used as part of tracking the total size of the utxo cache.
This adds a utxocachemaxsize configuration option which represents the maximum size in MiB of the utxo cache. The default value is 150 MiB, the minimum value is 25 MiB, and the maximum value is 32768 MiB (32 GiB).
This updates utxo viewpoints to deep copy the script when getting it from a msg tx. This is required since the tx out script is a subslice of the overall contiguous buffer that the msg tx houses for all scripts within the tx. It is deep copied here since this entry may be added to the utxo cache, and we don't want the utxo cache holding the entry to prevent all of the other tx scripts from getting garbage collected.
This adds a utxoSetState type that is tracked in the database. The utxo set state contains information regarding the current state of the utxo set. In particular, it tracks the block height and block hash of the last completed flush. The utxo set state is tracked in the database since at any given time, the utxo cache may not be consistent with the utxo set in the database. This is due to the fact that the utxo cache only flushes changes to the database periodically. Therefore, during initialization, the utxo set state is used to identify the last flushed state of the utxo set and it can be caught up to the current best state of the main chain. This additionally adds full test coverage to the new serialization and deserialization functions.
UtxoCache is an unspent transaction output cache that sits on top of the utxo set database and provides significant runtime performance benefits at the cost of some additional memory usage. It drastically reduces the amount of reading and writing to disk, especially during initial block download when a very large number of blocks are being processed in quick succession. The UtxoCache is a read-through cache. All utxo reads go through the cache. When there is a cache miss, the cache loads the missing data from the database, caches it, and returns it to the caller. The UtxoCache is a write-back cache. Writes to the cache are acknowledged by the cache immediately but are only periodically flushed to the database. This allows intermediate steps to effectively be skipped. For example, a utxo that is created and then spent in between flushes never needs to be written to the utxo set in the database. Due to the write-back nature of the cache, at any given time the database may not be in sync with the cache, and therefore all utxo reads and writes MUST go through the cache, and never read or write to the database directly. An overview of the changes is as follows: - Add UtxoCache and UtxoCacheConfig struct types and NewUtxoCache method - Update server to create the utxo cache with the configured max size and pass to the block chain instance that is created - Update all test block chains to create a utxo cache - Add FetchEntry to UtxoCache - FetchEntry returns the specified transaction output from the utxo set - If the output exists in the cache, it is returned immediately. Otherwise, it uses an existing database transaction to fetch the output from the database, caches it, and returns it to the caller. - Add AddEntry to UtxoCache - AddEntry adds the specified output to the cache - Add SpendEntry to UtxoCache - SpendEntry marks the specified output as spent - Remove entries that are marked as fresh and then subsequently spent. This is an optimization to skip writing to the database for outputs that are added and spent in between flushes to the database. - Update UtxoViewpoint to hold the UtxoCache - Update fetching entries from the database to fetch entries from the cache instead - Add Commit to UtxoCache - Commit updates all entries in the cache based on the state of each entry in the provided view - All entries in the provided view that are marked as modified and spent are removed from the view - Additionally, all entries that are added to the cache are removed from the provided view - Add MaybeFlush to UtxoCache - MaybeFlush conditionally flushes the cache to the database - If the maximum size of the cache has been reached, or if the periodic flush duration has been reached, then a flush is required - A flush can be forced by setting the force flush parameter - Flushing commits all modified entries to the database and conditionally evicts entries - Entries that are nil or spent are always evicted since they are unlikely to be accessed again. Additionally, if the cache has reached its maximum size, entries are evicted based on the height of the block that they are contained in. - Update connect block and disconnect block to commit to the cache and conditionally flush to the database - Rather than writing to the utxo set in the database every time that a block is connected or disconnected, commit the updated view to the cache and call MaybeFlush on the cache to conditionally flush it to the database - Add InitUtxoCache to UtxoCache - InitUtxoCache initializes the utxo cache by ensuring that the utxo set is caught up to the tip of the best chain - Since the cache is only flushed to the database periodically, the utxo set may not be caught up to the tip of the best chain - InitUtxoCache catches the utxo set up by replaying all blocks from the block after the block that was last flushed to the tip block through the cache - Add ShutdownUtxoCache to BlockChain - ShutdownUtxoCache flushes the utxo cache to the database on shutdown. Since the cache is flushed periodically during initial block download and flushed after every block is connected after initial block download is complete, this flush that occurs during shutdown should finish relatively quickly - Note that if an unclean shutdown occurs, the cache will still be initialized properly when restarted as during initialization it will replay blocks to catch up to the tip block if it was not fully flushed before shutting down. However, it is still preferred to flush when shutting down versus always recovering on startup since it is faster - Track the hit ratio of UtxoCache - Track the number of hits and misses when accessing the cache in order to calculate the overall hit ratio of the cache to gauge its performance
This adds full test coverage to the UtxoCache type and its methods. Additionally, since this uses the testing Cleanup function that was introduced in Go 1.14, this bumps the required go version for the blockchain package from 1.13 to 1.14.
This updates the minimum recommended memory (RAM) from 1GB to 2GB in the main README.md. The minimum recommended memory is being increased due to the introduction of the utxo cache.
NOTE: If you plan to test this, it is recommended to make a copy of your data directory first. Though this does not have a database migration, this changes the way that the utxo set is synced to the database and in the case of an unclean shutdown on this branch, the old code will not be able to recover.
UtxoCache
is an unspent transaction output cache that sits on top of the utxo set database and provides significant runtime performance benefits at the cost of some additional memory usage. It drastically reduces the amount of reading and writing to disk, especially during initial block download when a very large number of blocks are being processed in quick succession.Initial block download performance results are as follows. This testing was done syncing from a local peer (to avoid peer speed/latency variance) on a 2.4 GHz 8-Core w/ SSD machine (I don't have a HDD to test with currently but the performance improvement may be even more significant in that case since the speedup is primarily due to reducing reads/writes to disk):
UtxoCache
(w/ defaultutxocachemaxsize
of 150 MiB)The
UtxoCache
is a read-through cache. All utxo reads go through the cache. When there is a cache miss, the cache loads the missing data from the database, caches it, and returns it to the caller.The
UtxoCache
is a write-back cache. Writes to the cache are acknowledged by the cache immediately but are only periodically flushed to the database. This allows intermediate steps to effectively be skipped. For example, a utxo that is created and then spent in between flushes never needs to be written to the utxo set in the database.Due to the write-back nature of the cache, at any given time the database may not be in sync with the cache, and therefore all utxo reads and writes now go through the cache, and never read or write to the database directly.
This has been broken down into separate commits where possible to make the review process a bit easier since it is a fairly large changeset. An overview of the changes is as follows:
UtxoEntry
for the cacheutxoStateFresh
toUtxoEntry
, which indicates that an entry exists in the utxo cache but does not exist in theunderlying database
UtxoEntry
to be used as part of tracking the total size of the utxo cacheutxocachemaxsize
utxocachemaxsize
configuration option which represents the maximum size in MiB of the utxo cache. The default value is 150 MiB and the minimum value is 25 MiButxoSetState
to the databaseutxoSetState
type that is tracked in the database. The utxo set state contains information regarding the current state of the utxo set. In particular, it tracks the block height and block hash of the last completed flushUtxoCache
andUtxoCacheConfig
struct types andNewUtxoCache
methodFetchEntry
toUtxoCache
FetchEntry
returns the specified transaction output from the utxo setAddEntry
toUtxoCache
AddEntry
adds the specified output to the cacheSpendEntry
toUtxoCache
SpendEntry
marks the specified output as spentUtxoViewpoint
to hold theUtxoCache
Commit
toUtxoCache
Commit
updates all entries in the cache based on the state of each entry in the provided viewMaybeFlush
toUtxoCache
MaybeFlush
conditionally flushes the cache to the databaseMaybeFlush
on the cache to conditionally flush it to the databaseInitUtxoCache
toUtxoCache
InitUtxoCache
initializes the utxo cache by ensuring that the utxo set is caught up to the tip of the best chainInitUtxoCache
catches the utxo set up by replaying all blocks from the block after the block that was last flushed to the tip block through the cacheShutdownUtxoCache
toBlockChain
ShutdownUtxoCache
flushes the utxo cache to the database on shutdown. Since the cache is flushed periodically during initial block download and flushed after every block is connected after initial block download is complete, this flush that occurs during shutdown should finish relatively quicklyUtxoCache
UtxoCache
test coverageUtxoCache
type and its methodsUtxoCache
taking ownership of the entries from the view and avoiding additional allocations when adding the entries altogether.Part of #1145.