Merge branch 'topic/awelzel/topic/awelzel/2698-sqlite-backend-options'

* topic/awelzel/topic/awelzel/2698-sqlite-backend-options:
  broker/store: Extend SQLiteOptions around data safety and performance
  auxil/broker: Bump submodule

CHANGES

@@ -1,3 +1,16 @@
+5.2.0-dev.566 | 2023-01-30 11:05:40 +0100
+
+  * GH-2698: broker/store: Extend SQLiteOptions around data safety and performance (Arne Welzel, Corelight)
+
+    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.
+
 5.2.0-dev.563 | 2023-01-28 09:46:59 -0700
 
   * Enable Debian 10 builds for PRs (Tim Wojtulewicz, Corelight)

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
 ---------------------
 

VERSION

@@ -1 +1 @@
-5.2.0-dev.563
+5.2.0-dev.566

auxil/broker

@@ -1 +1 @@
-Subproject commit 955e0d0c6327f59c6ff68a009c65fa87e7a4ce87
+Subproject commit 4f60a0e2eeffc345a2ba80477b7c60e9e1c7dcb0

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 <https://www.sqlite.org/pragma.html#pragma_synchronous>`_
+		## 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 <https://www.sqlite.org/pragma.html#pragma_journal_mode>`_
+		## 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.

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<RecordVal>(0)->GetFieldAs<StringVal>(0)->CheckString();
+			auto sqlite_opts = options->GetField<RecordVal>("sqlite");
-			return {{"path", path}};
+			result["path"] = sqlite_opts->GetField<StringVal>("path")->CheckString();
+
+			if ( auto synchronous = sqlite_opts->GetField<EnumVal>("synchronous") )
+				result["synchronous"] = broker::enum_value(
+					sqlite_synchronous_type->Lookup(synchronous->Get()));
+
+			if ( auto journal_mode = sqlite_opts->GetField<EnumVal>("journal_mode") )
+				result["journal_mode"] = broker::enum_value(
+					sqlite_journal_mode_type->Lookup(journal_mode->Get()));
+
+			auto failure_mode = sqlite_opts->GetField<EnumVal>("failure_mode");
+			result["failure_mode"] = broker::enum_value(
+				failure_mode_type->Lookup(failure_mode->Get()));
+
+			auto integrity_check = sqlite_opts->GetField<BoolVal>("integrity_check")->Get();
+			result["integrity_check"] = integrity_check;
+
+			break;
 			}
 
 		default:
 			break;
 		}
 
-	return broker::backend_options{};
+	return result;
 	}
 
 	} // namespace zeek::Broker

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

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

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

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

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);
+	}

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);
+	}

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);
+	}

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);
+	}