Update badger sync settings to optimize memory usage during hypersync #636
Conversation
…ttings once hypersync completes
lib/server.go
Outdated
| func (srv *Server) updateDbOpts(opts badger.Options) { | ||
| // Make sure that a mempool process doesn't try to access the DB while we're closing and re-opening it. | ||
| srv.mempool.mtx.RLock() | ||
| defer srv.mempool.mtx.RUnlock() |
There was a problem hiding this comment.
You may actually want Lock() here not RLock(). RLock() is non-exclusive meaning that other threads could be reading the mempool db (but not writing to it). If you do Lock() it kicks out readers in addition to writers.
| defer srv.mempool.mtx.RUnlock() | ||
| // Make sure that a server process doesn't try to access the DB while we're closing and re-opening it. | ||
| srv.DbMutex.Lock() | ||
| defer srv.DbMutex.Unlock() |
There was a problem hiding this comment.
Hmm.. I'm a bit confused. I think DbMutex is only used here, which means that if anything else has a handle on the db then it won't exclude it from reading from that handle. So does this really do anything? It seems like the only thing it prevents is calling updateDbOpts in multiple threads, but nothing else.
There was a problem hiding this comment.
This is utilized in backend in a few recurring jobs, such as the hot feed routine.
There was a problem hiding this comment.
Hmm I see it's used in backend.
| srv.snapshot.mainDb = srv.blockchain.db | ||
| srv.mempool.bc.db = srv.blockchain.db | ||
| srv.mempool.backupUniversalUtxoView.Handle = srv.blockchain.db | ||
| srv.mempool.universalUtxoView.Handle = srv.blockchain.db |
There was a problem hiding this comment.
This is dirty but I'm trying to think about how we would clean it up. We could have a wrapper on the DB that this thing can update... but that's dirty too. Ideally you could change the db without having to update all of these handles manually... I'm noodling.
There was a problem hiding this comment.
Agreed, not exactly a pretty solution, but the only other option I could think of was the wrapper struct, which you've mentioned.
| // DbMutex protects the badger database from concurrent access when it's being closed & re-opened. | ||
| // This is necessary because the database is closed & re-opened when the node finishes hypersyncing in order | ||
| // to change the database options from Default options to Performance options. | ||
| DbMutex deadlock.Mutex |
There was a problem hiding this comment.
So we're doing a couple things here:
- We reduce the hypersync queue size. Seems like a strict win.
- We swap the db options on badger after we finish syncing. This one creates some messiness.
Is (2) a significant improvement to memory usage? Like if we merge (1) but not (2) does it go above 32gb?
There was a problem hiding this comment.
2 does win us a substantial amount of memory usage. The intense I/O requirements of hypersync married with our very high performance settings results in a massive amount of memory usage. In order to be reliably under 32GBs of usage, we do need to be using the default settings during HS.
An alternate approach here would be to fix the way we store utxo ops in the backend db. My hunch is that if we were to fix that index to only store a single utxo op per record, we would be able to run default settings in block sync, meaning that we could avoid the ugliness associated with stopping + restarting badger in order to switch the settings. This would involve moving the block + transaction to the key of that index, rather than having a single record that contains a bundle with every op for every transaction in a block.
Update hypersync to use default badger settings, switch to default settings once hypersync completes