Creating state-sync snapshot can fail with SQLITE_BUSY #8523

mhofman · 2023-11-10T01:10:20Z

Describe the bug

During blocks with a lot of activity, or while catching-up a node, taking a state-sync snapshot can often fail with an error code SQLITE_BUSY. Since the export is starting a read-only transaction, and the DB is in WAL mode, there is no reason it shouldn't be able to start the transaction.

To Reproduce

Enable state-sync snapshots on a node. Start from state-sync or another old cosmos snapshot.

Expected behavior

No errors starting a snapshot

Platform Environment

N/A

Additional context

SQLite likely needs to acquire a lock on the DB to indicate it's starting a read transaction, avoiding the flushing of the WAL file. It's possibly that lock acquiring that fails in some circumstances

Screenshots

2023-11-10T00:34:18.910338376Z State-sync export failed for block 12418000 { code: 'SQLITE_BUSY' }
2023-11-10T00:46:35.968256538Z State-sync export failed for block 12420000 { code: 'SQLITE_BUSY' }
2023-11-10T00:56:40.470466414Z State-sync export failed for block 12422000 { code: 'SQLITE_BUSY' }

The text was updated successfully, but these errors were encountered:

mhofman · 2023-11-10T01:42:24Z

Sources of info on how the WAL mode works and locking:

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 this this should fix the SQLITE_BUSY errors. 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

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

mhofman · 2023-12-06T04:10:41Z

Since the export is starting a read-only transaction, and the DB is in WAL mode, there is no reason it shouldn't be able to start the transaction.

For posterity, the problem was the openDB to get the block height, which started an immediate tx, and executed some statements that tried to kick the tx into write mode.

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

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

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

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 Agoric#8523

mhofman added bug Something isn't working cosmic-swingset package: cosmic-swingset swing-store labels Nov 10, 2023

mhofman mentioned this issue Nov 10, 2023

fix(swing-store): increase export timeout #8524

Closed

0xpatrickdev assigned 0xpatrickdev and unassigned 0xpatrickdev Nov 13, 2023

warner mentioned this issue Dec 5, 2023

feat: add exporter.getHostKV() API #8617

Closed

mhofman linked a pull request Dec 5, 2023 that will close this issue

feat: add exporter.getHostKV() API #8617

Closed

warner mentioned this issue Dec 5, 2023

fix: export state-sync snapshot without a DB write-lock #8619

Merged

mhofman linked a pull request Dec 5, 2023 that will close this issue

fix: export state-sync snapshot without a DB write-lock #8619

Merged

mergify bot closed this as completed in #8619 Dec 6, 2023

mhofman assigned warner Dec 6, 2023

mhofman removed a link to a pull request Dec 6, 2023

feat: add exporter.getHostKV() API #8617

Closed

mhofman mentioned this issue Dec 6, 2023

rebase/cherry-pick commits for upgrade 13 #8623

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Creating state-sync snapshot can fail with SQLITE_BUSY #8523

Creating state-sync snapshot can fail with SQLITE_BUSY #8523

mhofman commented Nov 10, 2023

mhofman commented Nov 10, 2023 •

edited

Loading

mhofman commented Dec 6, 2023

Creating state-sync snapshot can fail with SQLITE_BUSY #8523

Creating state-sync snapshot can fail with SQLITE_BUSY #8523

Comments

mhofman commented Nov 10, 2023

Describe the bug

To Reproduce

Expected behavior

Platform Environment

Additional context

Screenshots

mhofman commented Nov 10, 2023 • edited Loading

mhofman commented Dec 6, 2023

mhofman commented Nov 10, 2023 •

edited

Loading