From 0fbc325156d0c45d84e50be6b04e045363a0aebf Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Tue, 21 Mar 2023 11:09:59 +0100
Subject: [PATCH] sqlite: set connection attributes on open

The symptoms in #17859 indicate that setting the PRAGMAs in individual
EXECs outside of a transaction can lead to concurrency issues and
failures when the DB is locked.  Hence set all PRAGMAs when opening
the connection.  Move them into individual constants to improve
documentation and readability.

Further make transactions exclusive as #17859 also mentions an error
that the DB is locked during a transaction.

[NO NEW TESTS NEEDED] - existing tests cover the code.

Fixes: #17859
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>

<MH: Cherry-picked on top of my branch>

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
---
 libpod/sqlite_state.go | 40 +++++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go
index 7b9244913e..6f98668ec7 100644
--- a/libpod/sqlite_state.go
+++ b/libpod/sqlite_state.go
@@ -29,6 +29,30 @@ type SQLiteState struct {
 	runtime *Runtime
 }
 
+const (
+	// Deal with timezone automatically.
+	sqliteOptionLocation = "_loc=auto"
+	// Set the journal mode (https://www.sqlite.org/pragma.html#pragma_journal_mode).
+	sqliteOptionJournal = "&_journal=WAL"
+	// Force WAL mode to fsync after each transaction (https://www.sqlite.org/pragma.html#pragma_synchronous).
+	sqliteOptionSynchronous = "&_sync=FULL"
+	// Allow foreign keys (https://www.sqlite.org/pragma.html#pragma_foreign_keys).
+	sqliteOptionForeignKeys = "&_foreign_keys=1"
+	// Enable cache sharing for threads within a process
+	sqliteOptionSharedCache = "&cache=shared"
+	// Make sure that transactions happen exclusively.
+	sqliteOptionTXLock = "&_txlock=exclusive"
+
+	// Assembled sqlite options used when opening the database.
+	sqliteOptions = "db.sql?" +
+		sqliteOptionLocation +
+		sqliteOptionJournal +
+		sqliteOptionSynchronous +
+		sqliteOptionForeignKeys +
+		sqliteOptionSharedCache +
+		sqliteOptionTXLock
+)
+
 // NewSqliteState creates a new SQLite-backed state database.
 func NewSqliteState(runtime *Runtime) (_ State, defErr error) {
 	state := new(SQLiteState)
@@ -45,7 +69,7 @@ func NewSqliteState(runtime *Runtime) (_ State, defErr error) {
 		return nil, fmt.Errorf("creating root directory: %w", err)
 	}
 
-	conn, err := sql.Open("sqlite3", filepath.Join(basePath, "db.sql?_loc=auto&cache=shared"))
+	conn, err := sql.Open("sqlite3", filepath.Join(basePath, sqliteOptions))
 	if err != nil {
 		return nil, fmt.Errorf("initializing sqlite database: %w", err)
 	}
@@ -66,20 +90,6 @@ func NewSqliteState(runtime *Runtime) (_ State, defErr error) {
 		return nil, fmt.Errorf("cannot connect to database: %w", err)
 	}
 
-	// Enable foreign keys constraints, which we use extensively in our
-	// tables.
-	if _, err := state.conn.Exec("PRAGMA foreign_keys = ON;"); err != nil {
-		return nil, fmt.Errorf("enabling foreign key support in database: %w", err)
-	}
-	// Enable WAL mode for performance - https://www.sqlite.org/wal.html
-	if _, err := state.conn.Exec("PRAGMA journal_mode = WAL;"); err != nil {
-		return nil, fmt.Errorf("switching journal to WAL mode: %w", err)
-	}
-	// Force WAL mode to fsync after every transaction, for reboot safety.
-	if _, err := state.conn.Exec("PRAGMA synchronous = FULL;"); err != nil {
-		return nil, fmt.Errorf("setting full fsync mode in db: %w", err)
-	}
-
 	// Migrate schema (if necessary)
 	if err := state.migrateSchemaIfNecessary(); err != nil {
 		return nil, err