From bda95b27764eb07c69aa89e527e0a2d0b1036a79 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 08:03:08 +0000 Subject: [PATCH] Add _maxconn SQLite connection string parameter Lets the SQLite connection string set a custom max open connections via a "_maxconn=n" query parameter, since the driver doesn't support it natively. Zero or negative values fall back to the default, and the parameter is ignored for in-memory databases, which must stay capped at a single connection. --- backend/internal/bootstrap/db_bootstrap.go | 69 ++++++++++- .../internal/bootstrap/db_bootstrap_test.go | 109 ++++++++++++++++++ 2 files changed, 177 insertions(+), 1 deletion(-) diff --git a/backend/internal/bootstrap/db_bootstrap.go b/backend/internal/bootstrap/db_bootstrap.go index 01db6306..043931fe 100644 --- a/backend/internal/bootstrap/db_bootstrap.go +++ b/backend/internal/bootstrap/db_bootstrap.go @@ -6,6 +6,7 @@ import ( "fmt" "log/slog" "net/url" + "strconv" "strings" "time" @@ -58,10 +59,17 @@ func ConnectDatabase(ctx context.Context) (db *gorm.DB, pg *pgxpool.Pool, err er sqliteutil.RegisterSqliteFunctions() + // "_maxconn" is a Pocket ID-specific parameter, not understood by the SQLite driver, so it's extracted before the + // connection string is handed off + connString, maxConns, err := extractSqliteMaxConns(common.EnvConfig.DbConnectionString) + if err != nil { + return nil, nil, err + } + // The connector validates the connection string and performs the filesystem setup SQLite needs: it creates the database and temporary directories // It also warns when the database lives on a networked filesystem, which is unsupported connector, err := sqlitekit.NewConnector(sqlitekit.ConnectOpts{ - ConnString: addSqliteDatetimeParams(common.EnvConfig.DbConnectionString), + ConnString: addSqliteDatetimeParams(connString), Logger: slog.Default(), }) if err != nil { @@ -75,6 +83,11 @@ func ConnectDatabase(ctx context.Context) (db *gorm.DB, pg *pgxpool.Pool, err er return nil, nil, fmt.Errorf("failed to open SQLite database: %w", err) } + // Apply the "_maxconn" override, unless the database is in-memory: those must stay capped at 1 connection to see the whole data + if maxConns > 0 && !isSqliteInMemory(connString) { + sqliteDB.SetMaxOpenConns(maxConns) + } + dialector = sqlite.New(sqlite.Config{Conn: sqliteDB}) case common.DbProviderPostgres: if common.EnvConfig.DbConnectionString == "" { @@ -196,3 +209,57 @@ func addSqliteDatetimeParams(connString string) string { return path + "?" + qs.Encode() } + +// sqliteMaxConnParam is a Pocket ID-specific SQLite connection string parameter that sets the maximum number of connections in the pool. +const sqliteMaxConnParam = "_maxconn" + +// extractSqliteMaxConns removes the "_maxconn" parameter from a SQLite connection string, since the SQLite driver doesn't understand it, +// and returns the connection string without it along with the requested maximum number of pool connections. +// A value that's zero, negative, or absent means "use the default", represented here as maxConns == 0. +func extractSqliteMaxConns(connString string) (parsedConnString string, maxConns int, err error) { + path, rawQuery, found := strings.Cut(connString, "?") + if !found { + return connString, 0, nil + } + + qs, err := url.ParseQuery(rawQuery) + if err != nil { + // Return the connection string unmodified so the driver reports the parsing error + return connString, 0, nil + } + + v := qs.Get(sqliteMaxConnParam) + if v == "" { + return connString, 0, nil + } + qs.Del(sqliteMaxConnParam) + + n, err := strconv.Atoi(v) + if err != nil { + return "", 0, fmt.Errorf("invalid value for '%s' in the SQLite connection string: %w", sqliteMaxConnParam, err) + } + if n > 0 { + maxConns = n + } + + return path + "?" + qs.Encode(), maxConns, nil +} + +// isSqliteInMemory returns true if the SQLite connection string points to an in-memory database. +func isSqliteInMemory(connString string) bool { + lc := strings.ToLower(connString) + + // First way to define an in-memory database is to use ":memory:" or "file::memory:" as connection string + if strings.HasPrefix(lc, ":memory:") || strings.HasPrefix(lc, "file::memory:") { + return true + } + + // Another way is to pass "mode=memory" in the query string + _, rawQuery, found := strings.Cut(lc, "?") + if !found { + return false + } + + qs, _ := url.ParseQuery(rawQuery) + return len(qs["mode"]) > 0 && qs["mode"][0] == "memory" +} diff --git a/backend/internal/bootstrap/db_bootstrap_test.go b/backend/internal/bootstrap/db_bootstrap_test.go index 09f2c4fa..9d0437b9 100644 --- a/backend/internal/bootstrap/db_bootstrap_test.go +++ b/backend/internal/bootstrap/db_bootstrap_test.go @@ -120,6 +120,115 @@ func TestAddSqliteDatetimeParams(t *testing.T) { }) } +func TestExtractSqliteMaxConns(t *testing.T) { + tests := []struct { + name string + connString string + wantConnString string + wantMaxConns int + }{ + { + name: "no query string", + connString: "data/pocket-id.db", + wantConnString: "data/pocket-id.db", + wantMaxConns: 0, + }, + { + name: "_maxconn absent", + connString: "file:data/pocket-id.db?_txlock=immediate", + wantConnString: "file:data/pocket-id.db?_txlock=immediate", + wantMaxConns: 0, + }, + { + name: "_maxconn is applied and stripped", + connString: "file:data/pocket-id.db?_maxconn=5", + wantConnString: "file:data/pocket-id.db?", + wantMaxConns: 5, + }, + { + name: "_maxconn is stripped alongside other params", + connString: "file:data/pocket-id.db?_txlock=immediate&_maxconn=5", + wantConnString: "file:data/pocket-id.db?_txlock=immediate", + wantMaxConns: 5, + }, + { + name: "_maxconn=0 means use the default", + connString: "file:data/pocket-id.db?_maxconn=0", + wantConnString: "file:data/pocket-id.db?", + wantMaxConns: 0, + }, + { + name: "a negative _maxconn means use the default", + connString: "file:data/pocket-id.db?_maxconn=-5", + wantConnString: "file:data/pocket-id.db?", + wantMaxConns: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotConnString, gotMaxConns, err := extractSqliteMaxConns(tt.connString) + require.NoError(t, err) + assert.Equal(t, tt.wantMaxConns, gotMaxConns) + + wantPath, wantRawQuery, _ := strings.Cut(tt.wantConnString, "?") + gotPath, gotRawQuery, _ := strings.Cut(gotConnString, "?") + assert.Equal(t, wantPath, gotPath, "path was modified") + + wantQs, err := url.ParseQuery(wantRawQuery) + require.NoError(t, err) + gotQs, err := url.ParseQuery(gotRawQuery) + require.NoError(t, err) + assert.Equal(t, wantQs, gotQs) + }) + } + + t.Run("returns an error for a non-numeric value", func(t *testing.T) { + _, _, err := extractSqliteMaxConns("file:data/pocket-id.db?_maxconn=abc") + assert.Error(t, err) + }) +} + +func TestIsSqliteInMemory(t *testing.T) { + tests := []struct { + name string + connString string + want bool + }{ + { + name: "bare :memory: connection string", + connString: ":memory:", + want: true, + }, + { + name: "file::memory: URI", + connString: "file::memory:?cache=shared", + want: true, + }, + { + name: "mode=memory query parameter", + connString: "file:test.db?mode=memory", + want: true, + }, + { + name: "file-based database", + connString: "file:data/pocket-id.db", + want: false, + }, + { + name: "file-based database with unrelated query params", + connString: "file:data/pocket-id.db?mode=rwc", + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, isSqliteInMemory(tt.connString)) + }) + } +} + // TestConnectDatabaseSqlite checks that the connection Pocket ID now opens itself, so it can be instrumented, still behaves like the one Gorm used to open for us. // The datetime parameters are the part at risk: without them modernc.org/sqlite returns strings, not time.Time, for datetime columns. //