From 519d58d88c906abc5139e35933bdeba0396c10a2 Mon Sep 17 00:00:00 2001 From: "Alessandro (Ale) Segala" <43508+ItalyPaleAle@users.noreply.github.com> Date: Sat, 29 Mar 2025 15:12:48 -0700 Subject: [PATCH] fix: use WAL for SQLite by default and set busy_timeout (#388) Co-authored-by: Kyle Mendell --- .github/workflows/e2e-tests.yml | 2 +- backend/internal/bootstrap/bootstrap.go | 2 ++ .../internal/bootstrap/config_migration.go | 34 +++++++++++++++++++ backend/internal/bootstrap/db_bootstrap.go | 25 ++++++++++---- backend/internal/common/env_config.go | 16 +++++---- 5 files changed, 64 insertions(+), 15 deletions(-) create mode 100644 backend/internal/bootstrap/config_migration.go diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index cd812e47..0f731aab 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -152,7 +152,7 @@ jobs: -p 80:80 \ -e APP_ENV=test \ -e DB_PROVIDER=postgres \ - -e POSTGRES_CONNECTION_STRING=postgresql://postgres:postgres@pocket-id-db:5432/pocket-id \ + -e DB_CONNECTION_STRING=postgresql://postgres:postgres@pocket-id-db:5432/pocket-id \ pocket-id/pocket-id:test docker logs -f pocket-id-postgres &> /tmp/backend.log & diff --git a/backend/internal/bootstrap/bootstrap.go b/backend/internal/bootstrap/bootstrap.go index 824f8d33..c53e10e6 100644 --- a/backend/internal/bootstrap/bootstrap.go +++ b/backend/internal/bootstrap/bootstrap.go @@ -8,6 +8,8 @@ import ( func Bootstrap() { initApplicationImages() + migrateConfigDBConnstring() + db := newDatabase() appConfigService := service.NewAppConfigService(db) diff --git a/backend/internal/bootstrap/config_migration.go b/backend/internal/bootstrap/config_migration.go new file mode 100644 index 00000000..9cb51aaf --- /dev/null +++ b/backend/internal/bootstrap/config_migration.go @@ -0,0 +1,34 @@ +package bootstrap + +import ( + "log" + + "github.com/pocket-id/pocket-id/backend/internal/common" +) + +// Performs the migration of the database connection string +// See: https://github.com/pocket-id/pocket-id/pull/388 +func migrateConfigDBConnstring() { + switch common.EnvConfig.DbProvider { + case common.DbProviderSqlite: + // Check if we're using the deprecated SqliteDBPath env var + if common.EnvConfig.SqliteDBPath != "" { + connString := "file:" + common.EnvConfig.SqliteDBPath + "?_journal_mode=WAL&_busy_timeout=2500&_txlock=immediate" + common.EnvConfig.DbConnectionString = connString + common.EnvConfig.SqliteDBPath = "" + + log.Printf("[WARN] Env var 'SQLITE_DB_PATH' is deprecated - use 'DB_CONNECTION_STRING' instead with the value: '%s'", connString) + } + case common.DbProviderPostgres: + // Check if we're using the deprecated PostgresConnectionString alias + if common.EnvConfig.PostgresConnectionString != "" { + common.EnvConfig.DbConnectionString = common.EnvConfig.PostgresConnectionString + common.EnvConfig.PostgresConnectionString = "" + + log.Print("[WARN] Env var 'POSTGRES_CONNECTION_STRING' is deprecated - use 'DB_CONNECTION_STRING' instead with the same value") + } + default: + // We don't do anything here in the default case + // This is an error, but will be handled later on + } +} diff --git a/backend/internal/bootstrap/db_bootstrap.go b/backend/internal/bootstrap/db_bootstrap.go index 5fb588cf..c520d9cb 100644 --- a/backend/internal/bootstrap/db_bootstrap.go +++ b/backend/internal/bootstrap/db_bootstrap.go @@ -5,6 +5,7 @@ import ( "fmt" "log" "os" + "strings" "time" "github.com/golang-migrate/migrate/v4" @@ -38,6 +39,7 @@ func newDatabase() (db *gorm.DB) { case common.DbProviderPostgres: driver, err = postgresMigrate.WithInstance(sqlDb, &postgresMigrate.Config{}) default: + // Should never happen at this point log.Fatalf("unsupported database provider: %s", common.EnvConfig.DbProvider) } if err != nil { @@ -78,9 +80,18 @@ func connectDatabase() (db *gorm.DB, err error) { // Choose the correct database provider switch common.EnvConfig.DbProvider { case common.DbProviderSqlite: - dialector = sqlite.Open(common.EnvConfig.SqliteDBPath) + if common.EnvConfig.DbConnectionString == "" { + return nil, errors.New("missing required env var 'DB_CONNECTION_STRING' for SQLite database") + } + if !strings.HasPrefix(common.EnvConfig.DbConnectionString, "file:") { + return nil, errors.New("invalid value for env var 'DB_CONNECTION_STRING': does not begin with 'file:'") + } + dialector = sqlite.Open(common.EnvConfig.DbConnectionString) case common.DbProviderPostgres: - dialector = postgres.Open(common.EnvConfig.PostgresConnectionString) + if common.EnvConfig.DbConnectionString == "" { + return nil, errors.New("missing required env var 'DB_CONNECTION_STRING' for Postgres database") + } + dialector = postgres.Open(common.EnvConfig.DbConnectionString) default: return nil, fmt.Errorf("unsupported database provider: %s", common.EnvConfig.DbProvider) } @@ -91,14 +102,14 @@ func connectDatabase() (db *gorm.DB, err error) { Logger: getLogger(), }) if err == nil { - break - } else { - log.Printf("Attempt %d: Failed to initialize database. Retrying...", i) - time.Sleep(3 * time.Second) + return db, nil } + + log.Printf("Attempt %d: Failed to initialize database. Retrying...", i) + time.Sleep(3 * time.Second) } - return db, err + return nil, err } func getLogger() logger.Interface { diff --git a/backend/internal/common/env_config.go b/backend/internal/common/env_config.go index 5c03d8ba..e40e12b0 100644 --- a/backend/internal/common/env_config.go +++ b/backend/internal/common/env_config.go @@ -20,8 +20,9 @@ type EnvConfigSchema struct { AppEnv string `env:"APP_ENV"` AppURL string `env:"PUBLIC_APP_URL"` DbProvider DbProvider `env:"DB_PROVIDER"` - SqliteDBPath string `env:"SQLITE_DB_PATH"` - PostgresConnectionString string `env:"POSTGRES_CONNECTION_STRING"` + DbConnectionString string `env:"DB_CONNECTION_STRING"` + SqliteDBPath string `env:"SQLITE_DB_PATH"` // Deprecated: use "DB_CONNECTION_STRING" instead + PostgresConnectionString string `env:"POSTGRES_CONNECTION_STRING"` // Deprecated: use "DB_CONNECTION_STRING" instead UploadPath string `env:"UPLOAD_PATH"` KeysPath string `env:"KEYS_PATH"` Port string `env:"BACKEND_PORT"` @@ -35,7 +36,8 @@ type EnvConfigSchema struct { var EnvConfig = &EnvConfigSchema{ AppEnv: "production", DbProvider: "sqlite", - SqliteDBPath: "data/pocket-id.db", + DbConnectionString: "file:data/pocket-id.db?_journal_mode=WAL&_busy_timeout=2500&_txlock=immediate", + SqliteDBPath: "", PostgresConnectionString: "", UploadPath: "data/uploads", KeysPath: "data/keys", @@ -56,12 +58,12 @@ func init() { // Validate the environment variables switch EnvConfig.DbProvider { case DbProviderSqlite: - if EnvConfig.SqliteDBPath == "" { - log.Fatal("Missing SQLITE_DB_PATH environment variable") + if EnvConfig.DbConnectionString == "" { + log.Fatal("Missing required env var 'DB_CONNECTION_STRING' for SQLite database") } case DbProviderPostgres: - if EnvConfig.PostgresConnectionString == "" { - log.Fatal("Missing POSTGRES_CONNECTION_STRING environment variable") + if EnvConfig.DbConnectionString == "" { + log.Fatal("Missing required env var 'DB_CONNECTION_STRING' for Postgres database") } default: log.Fatal("Invalid DB_PROVIDER value. Must be 'sqlite' or 'postgres'")