From b80be48fed03cf5f813f9e44e0bcca5861540956 Mon Sep 17 00:00:00 2001
From: tobi <31960611+tsmethurst@users.noreply.github.com>
Date: Tue, 31 Jan 2023 13:46:45 +0100
Subject: [PATCH] [chore] Use 'immediate' lock for sqlite transactions (#1404)

* [chore] Use 'immediate' lock for sqlite transactions

* allow 1 connection regardless of cpu amount
---
 docs/configuration/database.md |  2 +-
 example/config.yaml            |  2 +-
 internal/db/bundb/bundb.go     | 48 +++++++++++++++++++++++++---------
 3 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/docs/configuration/database.md b/docs/configuration/database.md
index 4fe8836fd..758a01439 100644
--- a/docs/configuration/database.md
+++ b/docs/configuration/database.md
@@ -126,7 +126,7 @@ db-tls-ca-cert: ""
 # A multiplier of 8 is a sensible default, but you may wish to increase this for instances
 # running on very performant hardware, or decrease it for instances using v. slow CPUs.
 #
-# If you set this to 0 or less, it will be adjusted to 1.
+# If you set the multiplier to less than 1, only one open connection will be used regardless of cpu count.
 #
 # Examples: [16, 8, 10, 2]
 # Default: 8
diff --git a/example/config.yaml b/example/config.yaml
index 2f99fd326..f7d7c7884 100644
--- a/example/config.yaml
+++ b/example/config.yaml
@@ -182,7 +182,7 @@ db-tls-ca-cert: ""
 # A multiplier of 8 is a sensible default, but you may wish to increase this for instances
 # running on very performant hardware, or decrease it for instances using v. slow CPUs.
 #
-# If you set this to 0 or less, it will be adjusted to 1.
+# If you set the multiplier to less than 1, only one open connection will be used regardless of cpu count.
 #
 # Examples: [16, 8, 10, 2]
 # Default: 8
diff --git a/internal/db/bundb/bundb.go b/internal/db/bundb/bundb.go
index 385b0ca1f..6587ab8ad 100644
--- a/internal/db/bundb/bundb.go
+++ b/internal/db/bundb/bundb.go
@@ -256,16 +256,40 @@ func sqliteConn(ctx context.Context) (*DBConn, error) {
 	}
 
 	// Drop anything fancy from DB address
-	address = strings.Split(address, "?")[0]
-	address = strings.TrimPrefix(address, "file:")
+	address = strings.Split(address, "?")[0]       // drop any provided query strings
+	address = strings.TrimPrefix(address, "file:") // we'll prepend this later ourselves
 
-	// Append our own SQLite preferences
+	// build our own SQLite preferences
+	prefs := []string{
+		// use immediate transaction lock mode to fail quickly if tx can't lock
+		// see https://pkg.go.dev/modernc.org/sqlite#Driver.Open
+		"_txlock=immediate",
+	}
+
+	if address == ":memory:" {
+		log.Warn("using sqlite in-memory mode; all data will be deleted when gts shuts down; this mode should only be used for debugging or running tests")
+
+		// Use random name for in-memory instead of ':memory:', so
+		// multiple in-mem databases can be created without conflict.
+		address = uuid.NewString()
+
+		// in-mem-specific preferences
+		prefs = append(prefs, []string{
+			"mode=memory",  // indicate in-memory mode using query
+			"cache=shared", // shared cache so that tests don't fail
+		}...)
+	}
+
+	// rebuild address string with our derived preferences
 	address = "file:" + address
-
-	if address == "file::memory:" {
-		address = fmt.Sprintf("file:%s?mode=memory&cache=shared", uuid.NewString())
-		log.Infof("using in-memory database address " + address)
-		log.Warn("sqlite in-memory database should only be used for debugging")
+	for i, q := range prefs {
+		var prefix string
+		if i == 0 {
+			prefix = "?"
+		} else {
+			prefix = "&"
+		}
+		address += prefix + q
 	}
 
 	// Open new DB instance
@@ -274,7 +298,7 @@ func sqliteConn(ctx context.Context) (*DBConn, error) {
 		if errWithCode, ok := err.(*sqlite.Error); ok {
 			err = errors.New(sqlite.ErrorCodeString[errWithCode.Code()])
 		}
-		return nil, fmt.Errorf("could not open sqlite db: %s", err)
+		return nil, fmt.Errorf("could not open sqlite db with address %s: %w", address, err)
 	}
 
 	// Tune db connections for sqlite, see:
@@ -294,7 +318,7 @@ func sqliteConn(ctx context.Context) (*DBConn, error) {
 		}
 		return nil, fmt.Errorf("sqlite ping: %s", err)
 	}
-	log.Info("connected to SQLITE database")
+	log.Infof("connected to SQLITE database with address %s", address)
 
 	return conn, nil
 }
@@ -304,11 +328,11 @@ func sqliteConn(ctx context.Context) (*DBConn, error) {
 */
 
 // maxOpenConns returns multiplier * GOMAXPROCS,
-// clamping multiplier to 1 if it was below 1.
+// returning just 1 instead if multiplier < 1.
 func maxOpenConns() int {
 	multiplier := config.GetDbMaxOpenConnsMultiplier()
 	if multiplier < 1 {
-		multiplier = 1
+		return 1
 	}
 	return multiplier * runtime.GOMAXPROCS(0)
 }