feat: add db-pool-automatic-recovery configuration to disable connection retrying

CHANGELOG.md

@@ -5,6 +5,10 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
 ## Unreleased
 
+### Added
+ 
+ - #1614, Add `db-pool-automatic-recovery` configuration to disable connection retrying - @taimoorzaeem
+
 ### Fixed
 
  - #2899, Fix `application/vnd.pgrst.array` not accepted as a valid mediatype - @taimoorzaeem

src/PostgREST/AppState.hs

@@ -281,8 +281,9 @@ internalConnectionWorker appState = work
           -- Fatal error when connecting
           logWithZTime appState reason >> killThread (getMainThreadId appState)
         NotConnected ->
-          -- Unreachable because establishConnection will keep trying to connect
-          return ()
+          -- Unreachable because establishConnection will keep trying to connect, unless disable-recovery is turned on
+          unless configDbPoolAutomaticRecovery
+              $ logWithZTime appState "Automatic recovery disabled, exiting." >> killThread (getMainThreadId appState)
         Connected actualPgVersion -> do
           -- Procede with initialization
           putPgVersion appState actualPgVersion
@@ -344,9 +345,10 @@ establishConnection appState =
 
     shouldRetry :: RetryStatus -> ConnectionStatus -> IO Bool
     shouldRetry rs isConnSucc = do
+      AppConfig{..} <- getConfig appState
       let
         delay = fromMaybe 0 (rsPreviousDelay rs) `div` backoffMicroseconds
-        itShould = NotConnected == isConnSucc
+        itShould = NotConnected == isConnSucc && configDbPoolAutomaticRecovery
       when itShould . logWithZTime appState $
         "Attempting to reconnect to the database in "
         <> (show delay::Text)
@@ -420,7 +422,7 @@ listener appState = do
   waitListener appState
 
   -- forkFinally allows to detect if the thread dies
-  void . flip forkFinally (handleFinally dbChannel) $ do
+  void . flip forkFinally (handleFinally dbChannel configDbPoolAutomaticRecovery) $ do
     dbOrError <- acquire $ toUtf8 (addFallbackAppName prettyVersion configDbUri)
     case dbOrError of
       Right db -> do
@@ -431,7 +433,9 @@ listener appState = do
       _ ->
         die $ "Could not listen for notifications on the " <> dbChannel <> " channel"
   where
-    handleFinally dbChannel _ = do
+    handleFinally _ False _ =
+      logWithZTime appState "Automatic recovery disabled, exiting." >> killThread (getMainThreadId appState)
+    handleFinally dbChannel True _ = do
       -- if the thread dies, we try to recover
       logWithZTime appState $ "Retrying listening for notifications on the " <> dbChannel <> " channel.."
       putIsListenerOn appState False

src/PostgREST/CLI.hs

@@ -162,6 +162,9 @@ exampleConfigFile =
       |## Time in seconds after which to recycle unused pool connections
       |# db-pool-max-idletime = 30
       |
+      |## Allow autmatic database connection retrying
+      |# db-pool-automatic-recovery = true
+      |
       |## Stored proc to exec immediately after auth
       |# db-pre-request = "stored_proc_name"
       |

src/PostgREST/Config.hs

@@ -80,6 +80,7 @@ data AppConfig = AppConfig
   , configDbPoolAcquisitionTimeout :: Int
   , configDbPoolMaxLifetime        :: Int
   , configDbPoolMaxIdletime        :: Int
+  , configDbPoolAutomaticRecovery  :: Bool
   , configDbPreRequest             :: Maybe QualifiedIdentifier
   , configDbPreparedStatements     :: Bool
   , configDbRootSpec               :: Maybe QualifiedIdentifier
@@ -147,6 +148,7 @@ toText conf =
       ,("db-pool-acquisition-timeout",   show . configDbPoolAcquisitionTimeout)
       ,("db-pool-max-lifetime",          show . configDbPoolMaxLifetime)
       ,("db-pool-max-idletime",          show . configDbPoolMaxIdletime)
+      ,("db-pool-automatic-recovery",    T.toLower . show . configDbPoolAutomaticRecovery)
       ,("db-pre-request",            q . maybe mempty dumpQi . configDbPreRequest)
       ,("db-prepared-statements",        T.toLower . show . configDbPreparedStatements)
       ,("db-root-spec",              q . maybe mempty dumpQi . configDbRootSpec)
@@ -241,6 +243,7 @@ parser optPath env dbSettings roleSettings roleIsolationLvl =
     <*> (fromMaybe 1800 <$> optInt "db-pool-max-lifetime")
     <*> (fromMaybe 30 <$> optWithAlias (optInt "db-pool-timeout")
                                        (optInt "db-pool-max-idletime"))
+    <*> (fromMaybe True <$> optBool "db-pool-automatic-recovery")
     <*> (fmap toQi <$> optWithAlias (optString "db-pre-request")
                                     (optString "pre-request"))
     <*> (fromMaybe True <$> optBool "db-prepared-statements")

test/io/configs/expected/aliases.config

@@ -8,6 +8,7 @@ db-pool = 10
 db-pool-acquisition-timeout = 10
 db-pool-max-lifetime = 1800
 db-pool-max-idletime = 5
+db-pool-automatic-recovery = true
 db-pre-request = "check_alias"
 db-prepared-statements = true
 db-root-spec = "open_alias"

test/io/configs/expected/boolean-numeric.config

@@ -8,6 +8,7 @@ db-pool = 10
 db-pool-acquisition-timeout = 10
 db-pool-max-lifetime = 1800
 db-pool-max-idletime = 30
+db-pool-automatic-recovery = true
 db-pre-request = ""
 db-prepared-statements = false
 db-root-spec = ""

test/io/configs/expected/boolean-string.config

@@ -8,6 +8,7 @@ db-pool = 10
 db-pool-acquisition-timeout = 10
 db-pool-max-lifetime = 1800
 db-pool-max-idletime = 30
+db-pool-automatic-recovery = true
 db-pre-request = ""
 db-prepared-statements = false
 db-root-spec = ""

test/io/configs/expected/defaults.config

@@ -8,6 +8,7 @@ db-pool = 10
 db-pool-acquisition-timeout = 10
 db-pool-max-lifetime = 1800
 db-pool-max-idletime = 30
+db-pool-automatic-recovery = true
 db-pre-request = ""
 db-prepared-statements = true
 db-root-spec = ""

test/io/configs/expected/no-defaults-with-db-other-authenticator.config

@@ -8,6 +8,7 @@ db-pool = 1
 db-pool-acquisition-timeout = 30
 db-pool-max-lifetime = 3600
 db-pool-max-idletime = 60
+db-pool-automatic-recovery = false
 db-pre-request = "test.other_custom_headers"
 db-prepared-statements = false
 db-root-spec = "other_root"

test/io/configs/expected/no-defaults-with-db.config

@@ -8,6 +8,7 @@ db-pool = 1
 db-pool-acquisition-timeout = 30
 db-pool-max-lifetime = 3600
 db-pool-max-idletime = 60
+db-pool-automatic-recovery = false
 db-pre-request = "test.custom_headers"
 db-prepared-statements = false
 db-root-spec = "root"

test/io/configs/expected/no-defaults.config

@@ -8,6 +8,7 @@ db-pool = 1
 db-pool-acquisition-timeout = 30
 db-pool-max-lifetime = 3600
 db-pool-max-idletime = 60
+db-pool-automatic-recovery = false
 db-pre-request = "please_run_fast"
 db-prepared-statements = false
 db-root-spec = "openapi_v3"

test/io/configs/expected/types.config

@@ -8,6 +8,7 @@ db-pool = 10
 db-pool-acquisition-timeout = 10
 db-pool-max-lifetime = 1800
 db-pool-max-idletime = 30
+db-pool-automatic-recovery = true
 db-pre-request = ""
 db-prepared-statements = true
 db-root-spec = ""

test/io/configs/no-defaults-env.yaml

@@ -10,6 +10,7 @@ PGRST_DB_POOL: 1
 PGRST_DB_POOL_ACQUISITION_TIMEOUT: 30
 PGRST_DB_POOL_MAX_LIFETIME: 3600
 PGRST_DB_POOL_MAX_IDLETIME: 60
+PGRST_DB_POOL_AUTOMATIC_RECOVERY: false
 PGRST_DB_PREPARED_STATEMENTS: false
 PGRST_DB_PRE_REQUEST: please_run_fast
 PGRST_DB_ROOT_SPEC: openapi_v3

test/io/configs/no-defaults.config

@@ -8,6 +8,7 @@ db-pool = 1
 db-pool-acquisition-timeout = 30
 db-pool-max-lifetime = 3600
 db-pool-max-idletime = 60
+db-pool-automatic-recovery = false
 db-pre-request = "please_run_fast"
 db-prepared-statements = false
 db-root-spec = "openapi_v3"

test/io/fixtures.sql

@@ -173,4 +173,8 @@ select application_name
 from pg_stat_activity
 where application_name ilike 'postgrest%'
 limit 1;
-$$
+$$;
+
+create function terminate_pgrst() returns setof record as $$
+select pg_terminate_backend(pid) from pg_stat_activity where application_name iLIKE '%postgrest%';
+$$ language sql security definer;

test/io/test_io.py

@@ -1063,3 +1063,35 @@ def test_succeed_w_role_having_superuser_settings(defaultenv):
         response = postgrest.session.get("/projects", headers=headers)
         print(response.text)
         assert response.status_code == 200
+
+
+def test_fail_with_invalid_dbname_and_automatic_recovery_disabled(defaultenv):
+    "Should fail without retries when automatic recovery is disabled and dbname is invalid"
+    dbname = "INVALID"
+    uri = f'postgresql://?dbname={dbname}&host={defaultenv["PGHOST"]}&user={defaultenv["PGUSER"]}'
+    env = {
+        **defaultenv,
+        "PGRST_DB_URI": uri,
+        "PGRST_DB_POOL_AUTOMATIC_RECOVERY": "false",
+    }
+
+    with run(env=env, wait_for_readiness=False) as postgrest:
+        exitCode = wait_until_exit(postgrest)
+        assert exitCode == 1
+
+
+def test_fail_with_automatic_recovery_disabled_and_terminated_using_query(defaultenv):
+    "Should fail without retries when automatic recovery is disabled and pg_terminate_backend(pid) is called"
+
+    env = {
+        **defaultenv,
+        "PGRST_DB_POOL_AUTOMATIC_RECOVERY": "false",
+    }
+
+    with run(env=env) as postgrest:
+        os.system(
+            f'psql -d {defaultenv["PGDATABASE"]} -U {defaultenv["PGUSER"]} -h {defaultenv["PGHOST"]} --set ON_ERROR_STOP=1 -a -c "SELECT terminate_pgrst()"'
+        )
+
+        exitCode = wait_until_exit(postgrest)
+        assert exitCode == 1

test/spec/SpecHelper.hs

@@ -91,6 +91,7 @@ baseCfg = let secret = Just $ encodeUtf8 "reallyreallyreallyreallyverysafe" in
   , configDbPoolAcquisitionTimeout = 10
   , configDbPoolMaxLifetime     = 1800
   , configDbPoolMaxIdletime     = 600
+  , configDbPoolAutomaticRecovery = True
   , configDbPreRequest          = Just $ QualifiedIdentifier "test" "switch_role"
   , configDbPreparedStatements  = True
   , configDbRootSpec            = Nothing