fix: export state-sync snapshot without a DB write-lock #8619
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add a new API to the exporter, `exporter.getHostKV(key)`, so that the cosmic-swingset state-sync exporter process can query `host.height` without accidentally creating a read-write transaction too. refs #8523
Exporting a state-sync snapshot is a read-only operation, and is
designed to run "in the background", i.e. in parallel with normal
mutating operations. It accomplishes this by opening a read-only
transaction right away, effectively capturing a snapshot of the SQLite
database state, to insulate the export process from ongoing writes by
the execution host.
The cosmic-swingset exporter starts with a query of `host.height`, to
confirm that the database has not already advanced to a new block
before this snapshot/read-transaction can be taken.
Previously, this query worked by using `openSwingStore`, and then
calling `hostStorage.hostKVStore.get('host.height')`. This had two
problems:
* TOCTTOU: the `hostKVStore.get` used a different DB connection (and
different txn) than the exporter, so it might return a different
height, negating the accuracy of the consistency check
* read-write txn: `openSwingStore` creates a read-*write* txn, even
when merely opening the DB (because it might need to create the
initial tables). This txn is closed right away, before
`openSwingStore()` returns, so it did not present a threat to
ongoing operations. But if the exporter was created while the
ongoing execution side already had its own read-write txn
open (e.g. while `controller.run()` was running), then it would
fail, and `makeSwingStoreExporter` would fail with `SQLITE_BUSY`
Instead, we take advantage of the new `swingStoreExporter.getHostKV()`
API, and use *it* to fetch `host.height`. Unlike the normal
swingstore, the swingstore-exporter refrains from creating read-write
transactions entirely. So the cosmic-swingset export code can safely
query the height without fear of getting the wrong value or failing
because of an ongoing write transaction.
We think this should fix the SQLITE_BUSY errors.
refs #8523
To be accurate, that problem does not apply in production cases because cosmic-swingset, the caller of |
mhofman
approved these changes
Dec 5, 2023
|
Verified on a new patched mainnet follower node started from state-sync that producing a snapshot while syncing up after restore no longer triggers the SQLITE_BUSY error. |
mhofman
pushed a commit
that referenced
this pull request
Dec 6, 2023
…lock fix: export state-sync snapshot without a DB write-lock
mhofman
pushed a commit
that referenced
this pull request
Dec 6, 2023
…lock fix: export state-sync snapshot without a DB write-lock
mhofman
pushed a commit
that referenced
this pull request
Dec 6, 2023
…lock fix: export state-sync snapshot without a DB write-lock
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Exporting a state-sync snapshot is a read-only operation, and is designed to run "in the background", i.e. in parallel with normal mutating operations. It accomplishes this by opening a read-only transaction right away, effectively capturing a snapshot of the SQLite database state, to insulate the export process from ongoing writes by the execution host.
The cosmic-swingset exporter starts with a query of
host.height, to confirm that the database has not already advanced to a new block before this snapshot/read-transaction can be taken.Previously, this query worked by using
openSwingStore, and then callinghostStorage.hostKVStore.get('host.height'). This had two problems:TOCTTOU: the
hostKVStore.getused a different DB connection (and different txn) than the exporter, so it might return a different height, negating the accuracy of the consistency checkread-write txn:
openSwingStorecreates a read-write txn, even when merely opening the DB (because it might need to create the initial tables). This txn is closed right away, beforeopenSwingStore()returns, so it did not present a threat to ongoing operations. But if the exporter was created while the ongoing execution side already had its own read-write txn open (e.g. whilecontroller.run()was running), then it would fail, andmakeSwingStoreExporterwould fail withSQLITE_BUSYInstead, we take advantage of the new
swingStoreExporter.getHostKV()API, and use it to fetchhost.height. Unlike the normal swingstore, the swingstore-exporter refrains from creating read-write transactions entirely. So the cosmic-swingset export code can safely query the height without fear of getting the wrong value or failing because of an ongoing write transaction.We think this should fix the SQLITE_BUSY errors.
refs #8523