From 58d4b1564b23994b448c653f7e2dbcb58fb98c25 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>
---
 libpod/sqlite_state.go | 37 ++++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/libpod/sqlite_state.go b/libpod/sqlite_state.go
index 8317fe4b7c..68d1ea0fc5 100644
--- a/libpod/sqlite_state.go
+++ b/libpod/sqlite_state.go
@@ -29,6 +29,27 @@ 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"
+	// Make sure that transactions happen exclusively.
+	sqliteOptionTXLock = "&_txlock=exclusive"
+
+	// Assembled sqlite options used when opening the database.
+	sqliteOptions = "db.sql?" +
+		sqliteOptionLocation +
+		sqliteOptionJournal +
+		sqliteOptionSynchronous +
+		sqliteOptionForeignKeys +
+		sqliteOptionTXLock
+)
+
 // NewSqliteState creates a new SQLite-backed state database.
 func NewSqliteState(runtime *Runtime) (_ State, defErr error) {
 	state := new(SQLiteState)
@@ -45,7 +66,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"))
+	conn, err := sql.Open("sqlite3", filepath.Join(basePath, sqliteOptions))
 	if err != nil {
 		return nil, fmt.Errorf("initializing sqlite database: %w", err)
 	}
@@ -63,20 +84,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)
-	}
-
 	// Set up tables
 	if err := sqliteInitTables(state.conn); err != nil {
 		return nil, fmt.Errorf("creating tables: %w", err)