From f35cf228dc7ee7cb97a25789ab9f44e1e6eac38f Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Fri, 20 Jan 2023 16:15:23 +0100 Subject: [PATCH] broker/store: Extend SQLiteOptions around data safety and performance Add configurability of synchronous and journal_mode for SQLite backed Broker data stores. Setting these to synchronous=normal and journal_mode=wal can significantly improve throughput at the cost of some durability in the presence of power loss or OS crash. In the context of Zeek, this is likely more than acceptable. Additionally, add integrity_check and failure_mode options to support deleting and re-opening a corrupted SQLite database at store creation. Closes #2698 --- NEWS | 9 ++++ scripts/base/frameworks/broker/store.zeek | 50 +++++++++++++++++++ src/broker/Store.cc | 29 +++++++++-- .../out | 6 +++ .../out | 4 ++ .../out | 5 ++ .../out | 4 ++ ...erstore-backend-sqlite-corrupt-delete.zeek | 46 +++++++++++++++++ ...okerstore-backend-sqlite-corrupt-fail.zeek | 41 +++++++++++++++ ...rstore-backend-sqlite-integrity-check.zeek | 37 ++++++++++++++ .../brokerstore-backend-sqlite-wal-mode.zeek | 43 ++++++++++++++++ 11 files changed, 271 insertions(+), 3 deletions(-) create mode 100644 testing/btest/Baseline/broker.store.brokerstore-backend-sqlite-corrupt-delete/out create mode 100644 testing/btest/Baseline/broker.store.brokerstore-backend-sqlite-corrupt-fail/out create mode 100644 testing/btest/Baseline/broker.store.brokerstore-backend-sqlite-integrity-check/out create mode 100644 testing/btest/Baseline/broker.store.brokerstore-backend-sqlite-wal-mode/out create mode 100644 testing/btest/broker/store/brokerstore-backend-sqlite-corrupt-delete.zeek create mode 100644 testing/btest/broker/store/brokerstore-backend-sqlite-corrupt-fail.zeek create mode 100644 testing/btest/broker/store/brokerstore-backend-sqlite-integrity-check.zeek create mode 100644 testing/btest/broker/store/brokerstore-backend-sqlite-wal-mode.zeek diff --git a/NEWS b/NEWS index 99c062be19..cbe233e7ef 100644 --- a/NEWS +++ b/NEWS @@ -165,6 +165,15 @@ New Functionality of new analyzers as well as for collecting operational data in production environments. +- Expose configurability of for SQLite's synchronous and journal_mode PRAGMAs + for SQLite backed Broker data stores. Setting these to synchronous=normal + and journal_mode=wal can significantly improve throughput at the cost of + some durability in the presence of power loss or OS crash. In the context + of Zeek, this is likely more than acceptable. + + Additionally, add integrity_check and failure_mode options to support + detecting and deleting corrupted SQLite database at store initialization. + Changed Functionality --------------------- diff --git a/scripts/base/frameworks/broker/store.zeek b/scripts/base/frameworks/broker/store.zeek index 7c21407a3a..e5b0e14da8 100644 --- a/scripts/base/frameworks/broker/store.zeek +++ b/scripts/base/frameworks/broker/store.zeek @@ -57,12 +57,62 @@ export { SQLITE, }; + ## Behavior when the SQLite database file is found to be corrupt + ## or otherwise fails to open or initialize. + type SQLiteFailureMode: enum { + SQLITE_FAILURE_MODE_FAIL, ##< Fail during initialization. + SQLITE_FAILURE_MODE_DELETE, ##< Attempt to delete the database file and retry. + }; + + ## Values supported for SQLite's PRAGMA synchronous statement. + type SQLiteSynchronous: enum { + SQLITE_SYNCHRONOUS_OFF, + SQLITE_SYNCHRONOUS_NORMAL, + SQLITE_SYNCHRONOUS_FULL, + SQLITE_SYNCHRONOUS_EXTRA, + }; + + ## Values supported for SQLite's PRAGMA journal_mode statement. + type SQLiteJournalMode: enum { + SQLITE_JOURNAL_MODE_DELETE, + SQLITE_JOURNAL_MODE_WAL, + }; + ## Options to tune the SQLite storage backend. type SQLiteOptions: record { ## File system path of the database. ## If left empty, will be derived from the name of the store, ## and use the '.sqlite' file suffix. path: string &default = ""; + + ## If set, runs the PRAGMA synchronous statement with the + ## provided value after connecting to the SQLite database. See + ## `SQLite's synchronous documentation `_ + ## for more details around performance and data safety trade offs. + synchronous: SQLiteSynchronous &optional; + + ## If set, runs the PRAGMA journal_mode statement with the + ## provided value after connecting to the SQLite database. See + ## `SQLite's journal_mode documentation `_ + ## for more details around performance, data safety trade offs + ## and interaction with the PRAGMA synchronous statement. + journal_mode: SQLiteJournalMode &optional; + + ## What to do when the database is found corrupt during + ## initialization. When set to SQLITE_FAILURE_MODE_DELETE, + ## the old file is deleted to allow creation of a new and empty + ## database. By default, an error is reported, the corrupt + ## database file left in place and the data store is in a + ## non-functional state. + failure_mode: SQLiteFailureMode &default=SQLITE_FAILURE_MODE_FAIL; + + ## When true, run the PRAGMA integrity_check statement after + ## opening the database and fail according to ``failure_mode``. + ## PRAGMA integrity_check may take a non-negligible amount of time, + ## so you are advised to experiment with the expected sizes + ## of your databases if that is acceptable. Corrupted databases + ## should be reliably detected when this setting is ``T``. + integrity_check: bool &default=F; }; ## Options to tune the particular storage backends. diff --git a/src/broker/Store.cc b/src/broker/Store.cc index d548657a17..869a1ec893 100644 --- a/src/broker/Store.cc +++ b/src/broker/Store.cc @@ -66,19 +66,42 @@ broker::backend to_backend_type(BifEnum::Broker::BackendType type) broker::backend_options to_backend_options(broker::backend backend, RecordVal* options) { + static auto failure_mode_type = id::find_type("Broker::SQLiteFailureMode")->AsEnumType(); + static auto sqlite_synchronous_type = id::find_type("Broker::SQLiteSynchronous")->AsEnumType(); + static auto sqlite_journal_mode_type = id::find_type("Broker::SQLiteJournalMode")->AsEnumType(); + + broker::backend_options result; + switch ( backend ) { case broker::backend::sqlite: { - auto path = options->GetFieldAs(0)->GetFieldAs(0)->CheckString(); - return {{"path", path}}; + auto sqlite_opts = options->GetField("sqlite"); + result["path"] = sqlite_opts->GetField("path")->CheckString(); + + if ( auto synchronous = sqlite_opts->GetField("synchronous") ) + result["synchronous"] = broker::enum_value( + sqlite_synchronous_type->Lookup(synchronous->Get())); + + if ( auto journal_mode = sqlite_opts->GetField("journal_mode") ) + result["journal_mode"] = broker::enum_value( + sqlite_journal_mode_type->Lookup(journal_mode->Get())); + + auto failure_mode = sqlite_opts->GetField("failure_mode"); + result["failure_mode"] = broker::enum_value( + failure_mode_type->Lookup(failure_mode->Get())); + + auto integrity_check = sqlite_opts->GetField("integrity_check")->Get(); + result["integrity_check"] = integrity_check; + + break; } default: break; } - return broker::backend_options{}; + return result; } } // namespace zeek::Broker diff --git a/testing/btest/Baseline/broker.store.brokerstore-backend-sqlite-corrupt-delete/out b/testing/btest/Baseline/broker.store.brokerstore-backend-sqlite-corrupt-delete/out new file mode 100644 index 0000000000..e9b5299f9c --- /dev/null +++ b/testing/btest/Baseline/broker.store.brokerstore-backend-sqlite-corrupt-delete/out @@ -0,0 +1,6 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +store is open +populated 100 rows +store is open +populated 100 rows +100 diff --git a/testing/btest/Baseline/broker.store.brokerstore-backend-sqlite-corrupt-fail/out b/testing/btest/Baseline/broker.store.brokerstore-backend-sqlite-corrupt-fail/out new file mode 100644 index 0000000000..be127af724 --- /dev/null +++ b/testing/btest/Baseline/broker.store.brokerstore-backend-sqlite-corrupt-fail/out @@ -0,0 +1,4 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +store is open +populated 100 rows +failed to open store diff --git a/testing/btest/Baseline/broker.store.brokerstore-backend-sqlite-integrity-check/out b/testing/btest/Baseline/broker.store.brokerstore-backend-sqlite-integrity-check/out new file mode 100644 index 0000000000..1791275f5c --- /dev/null +++ b/testing/btest/Baseline/broker.store.brokerstore-backend-sqlite-integrity-check/out @@ -0,0 +1,5 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +store is open +populated 100 rows +store is open +populated 100 rows diff --git a/testing/btest/Baseline/broker.store.brokerstore-backend-sqlite-wal-mode/out b/testing/btest/Baseline/broker.store.brokerstore-backend-sqlite-wal-mode/out new file mode 100644 index 0000000000..8f978ce99f --- /dev/null +++ b/testing/btest/Baseline/broker.store.brokerstore-backend-sqlite-wal-mode/out @@ -0,0 +1,4 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +populated 100 rows +wal +100 diff --git a/testing/btest/broker/store/brokerstore-backend-sqlite-corrupt-delete.zeek b/testing/btest/broker/store/brokerstore-backend-sqlite-corrupt-delete.zeek new file mode 100644 index 0000000000..0df00a5a18 --- /dev/null +++ b/testing/btest/broker/store/brokerstore-backend-sqlite-corrupt-delete.zeek @@ -0,0 +1,46 @@ +# @TEST-DOC: Populate a database, corrupt it then observe Zeek's behavior deleting the database and reopening it. +# @TEST-REQUIRES: dd --version +# @TEST-REQUIRES: sqlite3 --version +# @TEST-REQUIRES: test -e /dev/zero +# @TEST-EXEC: zeek -b %INPUT >> out + +# Evil +# @TEST-EXEC: dd if=/dev/zero of=path_to_db.sqlite seek=512 count=32 bs=1 +# @TEST-EXEC: zeek -b %INPUT >> out + +# This will find 100 rows, the previous DB was deleted. +# @TEST-EXEC: sqlite3 ./path_to_db.sqlite 'select count(*) from store' >> out; +# +# @TEST-EXEC: grep 'database disk image is malformed' .stderr +# @TEST-EXEC: btest-diff out + +@load base/frameworks/broker/store + +global test_store: opaque of Broker::Store; +global test_table: table[string] of count &broker_store="test_store_42"; + +event zeek_init() + { + test_store = Broker::create_master( + "test_store_42", + Broker::SQLITE, + Broker::BackendOptions( + $sqlite=Broker::SQLiteOptions( + $path="path_to_db.sqlite", + $failure_mode=Broker::SQLITE_FAILURE_MODE_DELETE, + ), + ), + ); + if ( Broker::is_closed(test_store) ) { + print("failed to open store"); + exit(1); + } else { + print("store is open"); + } + + local rows = 100; + local i = 0; + while ( ++i <= rows ) + test_table[cat(|test_table|)] = i; + print fmt("populated %s rows", rows); + } diff --git a/testing/btest/broker/store/brokerstore-backend-sqlite-corrupt-fail.zeek b/testing/btest/broker/store/brokerstore-backend-sqlite-corrupt-fail.zeek new file mode 100644 index 0000000000..c0631f3ead --- /dev/null +++ b/testing/btest/broker/store/brokerstore-backend-sqlite-corrupt-fail.zeek @@ -0,0 +1,41 @@ +# @TEST-DOC: Populate a database, corrupt it then observe Zeek's behavior not being able to open the database and store. +# @TEST-REQUIRES: dd --version +# @TEST-REQUIRES: test -e /dev/zero +# @TEST-EXEC: zeek -b %INPUT >> out + +# Evil +# @TEST-EXEC: dd if=/dev/zero of=path_to_db.sqlite seek=512 count=32 bs=1 +# @TEST-EXEC-FAIL: zeek -b %INPUT >> out +# +# @TEST-EXEC: grep 'database disk image is malformed' .stderr +# @TEST-EXEC: btest-diff out + +@load base/frameworks/broker/store + +global test_store: opaque of Broker::Store; +global test_table: table[string] of count &broker_store="test_store_42"; + +event zeek_init() + { + test_store = Broker::create_master( + "test_store_42", + Broker::SQLITE, + Broker::BackendOptions( + $sqlite=Broker::SQLiteOptions( + $path="path_to_db.sqlite", + ), + ), + ); + if ( Broker::is_closed(test_store) ) { + print("failed to open store"); + exit(1); + } else { + print("store is open"); + } + + local rows = 100; + local i = 0; + while ( ++i <= rows ) + test_table[cat(|test_table|)] = i; + print fmt("populated %s rows", rows); + } diff --git a/testing/btest/broker/store/brokerstore-backend-sqlite-integrity-check.zeek b/testing/btest/broker/store/brokerstore-backend-sqlite-integrity-check.zeek new file mode 100644 index 0000000000..b3d905dcac --- /dev/null +++ b/testing/btest/broker/store/brokerstore-backend-sqlite-integrity-check.zeek @@ -0,0 +1,37 @@ +# @TEST-DOC: Use SQLite backend option integrity_check, but not breaking anything. + +# @TEST-EXEC: zeek -b %INPUT >> out +# @TEST-EXEC: zeek -b %INPUT >> out + +# @TEST-EXEC: btest-diff out + +@load base/frameworks/broker/store + +global test_store: opaque of Broker::Store; +global test_table: table[string] of count &broker_store="test_store_42"; + +event zeek_init() + { + test_store = Broker::create_master( + "test_store_42", + Broker::SQLITE, + Broker::BackendOptions( + $sqlite=Broker::SQLiteOptions( + $path="path_to_db.sqlite", + $integrity_check=T, + ), + ), + ); + if ( Broker::is_closed(test_store) ) { + print("failed to open store"); + exit(1); + } else { + print("store is open"); + } + + local rows = 100; + local i = 0; + while ( ++i <= rows ) + test_table[cat(|test_table|)] = i; + print fmt("populated %s rows", rows); + } diff --git a/testing/btest/broker/store/brokerstore-backend-sqlite-wal-mode.zeek b/testing/btest/broker/store/brokerstore-backend-sqlite-wal-mode.zeek new file mode 100644 index 0000000000..566ffc3386 --- /dev/null +++ b/testing/btest/broker/store/brokerstore-backend-sqlite-wal-mode.zeek @@ -0,0 +1,43 @@ +# @TEST-DOC: Configure a broker store to be in WAL mode withou journal_mode NORMAL. +# @TEST-REQUIRES: sqlite3 --version +# @TEST-EXEC: zeek -b %INPUT > out 2>&1 +# +# This is poking a bit at SQLite internals, but because WAL mode +# was flipped on, expect a wal and a shm file to exist. +# @TEST-EXEC: test -f path_to_db.sqlite || ls -lha >> out +# @TEST-EXEC: test -f path_to_db.sqlite-shm || ls -lha >> out +# @TEST-EXEC: test -f path_to_db.sqlite-wal || ls -lha >> out + +# More poking, running sqlite3 should detect WAL mode, and the store +# table has 100 entries. +# +# @TEST-EXEC: sqlite3 ./path_to_db.sqlite 'PRAGMA journal_mode' >> out; +# @TEST-EXEC: sqlite3 ./path_to_db.sqlite 'select count(*) from store' >> out; +# +# @TEST-EXEC: btest-diff out + +@load base/frameworks/broker/store + +global test_store: opaque of Broker::Store; +global test_table: table[string] of count &broker_store="test_store_42"; + +event zeek_init() + { + test_store = Broker::create_master( + "test_store_42", + Broker::SQLITE, + Broker::BackendOptions( + $sqlite=Broker::SQLiteOptions( + $path="path_to_db.sqlite", + $synchronous=Broker::SQLITE_SYNCHRONOUS_NORMAL, + $journal_mode=Broker::SQLITE_JOURNAL_MODE_WAL, + ), + ), + ); + + local rows = 100; + local i = 0; + while ( ++i <= rows ) + test_table[cat(|test_table|)] = i; + print fmt("populated %s rows", rows); + }