Skip to content

Commit

Permalink
feat: use hasql-pool-0.9, add db-pool-max-lifetime (fixes #2638)
Browse files Browse the repository at this point in the history
- db-pool-acquisition-timeout is no longer optional, defaults to 10s
- new option db-pool-max-lifetime limits the maximal lifetime of a
  postgresql connection, defaults to 30m
  • Loading branch information
robx authored and steve-chavez committed Apr 12, 2023
1 parent b869dd7 commit f26cdd5
Show file tree
Hide file tree
Showing 20 changed files with 83 additions and 23 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ This project adheres to [Semantic Versioning](http://semver.org/).

## Unreleased

### Added

- #2663, Limit maximal postgresql connection lifetime - @robx
+ New option `db-pool-max-lifetime` (default 30m)
+ `db-pool-acquisition-timeout` is no longer optional and defaults to 10s
+ Fixes postgresql resource leak with long-lived connections (#2638)

### Fixed

- #2667, Fix `db-pool-acquisition-timeout` not logging to stderr when the timeout is reached - @steve-chavez
Expand Down
18 changes: 18 additions & 0 deletions nix/overlays/haskell-packages.nix
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,24 @@ let
sha256 = "1wmyhldk0k14y8whp1p4akrkqxf5snh8qsbm7fv5f7kz95nyffd0";
})
{ });

hasql-notifications = lib.dontCheck
(prev.callHackageDirect
{
pkg = "hasql-notifications";
ver = "0.2.0.4";
sha256 = "sha256-fm1xiDyvDkb5WLOJ73/s8wrWEW23XFS7luAv2brfr8I=";
}
{ });

hasql-pool = lib.dontCheck
(prev.callHackageDirect
{
pkg = "hasql-pool";
ver = "0.9";
sha256 = "sha256-5UshbbaBVY8eJ/9VagNVVxonRwMcd7UmGqDc35pJNFY=";
}
{ });
} // extraOverrides final prev;
in
{
Expand Down
6 changes: 3 additions & 3 deletions postgrest.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ library
, hasql >= 1.6.1.1 && < 1.7
, hasql-dynamic-statements >= 0.3.1 && < 0.4
, hasql-notifications >= 0.1 && < 0.3
, hasql-pool >= 0.8.0.6 && < 0.9
, hasql-pool >= 0.9 && < 0.10
, hasql-transaction >= 1.0.1 && < 1.1
, heredoc >= 0.2 && < 0.3
, http-types >= 0.12.2 && < 0.13
Expand Down Expand Up @@ -228,7 +228,7 @@ test-suite spec
, bytestring >= 0.10.8 && < 0.12
, case-insensitive >= 1.2 && < 1.3
, containers >= 0.5.7 && < 0.7
, hasql-pool >= 0.8.0.2 && < 0.9
, hasql-pool >= 0.9 && < 0.10
, hasql-transaction >= 1.0.1 && < 1.1
, heredoc >= 0.2 && < 0.3
, hspec >= 2.3 && < 2.10
Expand All @@ -247,7 +247,7 @@ test-suite spec
, transformers-base >= 0.4.4 && < 0.5
, wai >= 3.2.1 && < 3.3
, wai-extra >= 3.0.19 && < 3.2
ghc-options: -O0 -Werror -Wall -fwarn-identities
ghc-options: -threaded -O0 -Werror -Wall -fwarn-identities
-fno-spec-constr -optP-Wno-nonportable-include-path
-fno-warn-missing-signatures
-fwrite-ide-info
Expand Down
9 changes: 5 additions & 4 deletions src/PostgREST/AppState.hs
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,11 @@ destroy = destroyPool

initPool :: AppConfig -> IO SQL.Pool
initPool AppConfig{..} =
SQL.acquire configDbPoolSize timeoutMilliseconds $ toUtf8 configDbUri
where
timeoutMilliseconds = (* oneSecond) <$> configDbPoolAcquisitionTimeout
oneSecond = 1000000
SQL.acquire
configDbPoolSize
(fromIntegral configDbPoolAcquisitionTimeout)
(fromIntegral configDbPoolMaxLifetime)
(toUtf8 configDbUri)

-- | Run an action with a database connection.
usePool :: AppState -> SQL.Session a -> IO (Either SQL.UsageError a)
Expand Down
3 changes: 3 additions & 0 deletions src/PostgREST/CLI.hs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,9 @@ exampleConfigFile =
|## Time in seconds to wait to acquire a slot from the connection pool
|# db-pool-acquisition-timeout = 10
|
|## Time in seconds after which to recycle pool connections
|# db-pool-max-lifetime = 1800
|
|## Stored proc to exec immediately after auth
|# db-pre-request = "stored_proc_name"
|
Expand Down
12 changes: 8 additions & 4 deletions src/PostgREST/Config.hs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ data AppConfig = AppConfig
, configDbMaxRows :: Maybe Integer
, configDbPlanEnabled :: Bool
, configDbPoolSize :: Int
, configDbPoolAcquisitionTimeout :: Maybe Int
, configDbPoolAcquisitionTimeout :: Int
, configDbPoolMaxLifetime :: Int
, configDbPreRequest :: Maybe QualifiedIdentifier
, configDbPreparedStatements :: Bool
, configDbRootSpec :: Maybe QualifiedIdentifier
Expand Down Expand Up @@ -130,7 +131,8 @@ toText conf =
,("db-max-rows", maybe "\"\"" show . configDbMaxRows)
,("db-plan-enabled", T.toLower . show . configDbPlanEnabled)
,("db-pool", show . configDbPoolSize)
,("db-pool-acquisition-timeout", maybe "\"\"" show . configDbPoolAcquisitionTimeout)
,("db-pool-acquisition-timeout", show . configDbPoolAcquisitionTimeout)
,("db-pool-max-lifetime", show . configDbPoolMaxLifetime)
,("db-pre-request", q . maybe mempty dumpQi . configDbPreRequest)
,("db-prepared-statements", T.toLower . show . configDbPreparedStatements)
,("db-root-spec", q . maybe mempty dumpQi . configDbRootSpec)
Expand Down Expand Up @@ -219,7 +221,8 @@ parser optPath env dbSettings =
(optInt "max-rows")
<*> (fromMaybe False <$> optBool "db-plan-enabled")
<*> (fromMaybe 10 <$> optInt "db-pool")
<*> optInt "db-pool-acquisition-timeout"
<*> (fromMaybe 10 <$> optInt "db-pool-acquisition-timeout")
<*> (fromMaybe 1800 <$> optInt "db-pool-max-lifetime")
<*> (fmap toQi <$> optWithAlias (optString "db-pre-request")
(optString "pre-request"))
<*> (fromMaybe True <$> optBool "db-prepared-statements")
Expand Down Expand Up @@ -355,7 +358,8 @@ parser optPath env dbSettings =
let dbSettingName = T.pack $ dashToUnderscore <$> toS key in
if dbSettingName `notElem` [
"server_host", "server_port", "server_unix_socket", "server_unix_socket_mode", "admin_server_port", "log_level",
"db_uri", "db_channel_enabled", "db_channel", "db_pool", "db_pool_acquisition_timeout", "db_config"]
"db_uri", "db_channel_enabled", "db_channel", "db_pool", "db_pool_acquisition_timeout",
"db_pool_max_lifetime", "db_config"]
then lookup dbSettingName dbSettings
else Nothing

Expand Down
2 changes: 2 additions & 0 deletions stack.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,5 @@ nix:
extra-deps:
- git: https://github.com/PostgREST/postgresql-libpq.git
commit: 890a0a16cf57dd401420fdc6c7d576fb696003bc
- hasql-notifications-0.2.0.4
- hasql-pool-0.9
14 changes: 14 additions & 0 deletions stack.yaml.lock
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,20 @@ packages:
original:
commit: 890a0a16cf57dd401420fdc6c7d576fb696003bc
git: https://github.com/PostgREST/postgresql-libpq.git
- completed:
hackage: hasql-notifications-0.2.0.4@sha256:9a09fa9b97feadd9492c8bd8bc6b9cffe0513510102f08374b0c45ecd479ed67,2028
pantry-tree:
sha256: 56f9e240728e7a65711dde45fa2e2075b914e32cd370424aaa4572392378a60e
size: 452
original:
hackage: hasql-notifications-0.2.0.4
- completed:
hackage: hasql-pool-0.9@sha256:db7a37f6b3a922c37adc3c7ced47a7c10786d1f171e47a735a6e812a587ba44c,2111
pantry-tree:
sha256: 49b1181d28c6f5317e794671c2dae155754b834bdcfa30f7e5dbad28e4cf0249
size: 346
original:
hackage: hasql-pool-0.9
snapshots:
- completed:
sha256: 4905c93319aa94aa53da8f41d614d7bacdbfe6c63a8c6132d32e6e62f24a9af4
Expand Down
3 changes: 2 additions & 1 deletion test/io/configs/expected/aliases.config
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ db-extra-search-path = "public"
db-max-rows = 1000
db-plan-enabled = false
db-pool = 10
db-pool-acquisition-timeout = ""
db-pool-acquisition-timeout = 10
db-pool-max-lifetime = 1800
db-pre-request = "check_alias"
db-prepared-statements = true
db-root-spec = "open_alias"
Expand Down
3 changes: 2 additions & 1 deletion test/io/configs/expected/boolean-numeric.config
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ db-extra-search-path = "public"
db-max-rows = ""
db-plan-enabled = false
db-pool = 10
db-pool-acquisition-timeout = ""
db-pool-acquisition-timeout = 10
db-pool-max-lifetime = 1800
db-pre-request = ""
db-prepared-statements = false
db-root-spec = ""
Expand Down
3 changes: 2 additions & 1 deletion test/io/configs/expected/boolean-string.config
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ db-extra-search-path = "public"
db-max-rows = ""
db-plan-enabled = false
db-pool = 10
db-pool-acquisition-timeout = ""
db-pool-acquisition-timeout = 10
db-pool-max-lifetime = 1800
db-pre-request = ""
db-prepared-statements = false
db-root-spec = ""
Expand Down
3 changes: 2 additions & 1 deletion test/io/configs/expected/defaults.config
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ db-extra-search-path = "public"
db-max-rows = ""
db-plan-enabled = false
db-pool = 10
db-pool-acquisition-timeout = ""
db-pool-acquisition-timeout = 10
db-pool-max-lifetime = 1800
db-pre-request = ""
db-prepared-statements = true
db-root-spec = ""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ db-extra-search-path = "public,extensions,other"
db-max-rows = 100
db-plan-enabled = true
db-pool = 1
db-pool-acquisition-timeout = 10
db-pool-acquisition-timeout = 30
db-pool-max-lifetime = 3600
db-pre-request = "test.other_custom_headers"
db-prepared-statements = false
db-root-spec = "other_root"
Expand Down
3 changes: 2 additions & 1 deletion test/io/configs/expected/no-defaults-with-db.config
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ db-extra-search-path = "public,extensions,private"
db-max-rows = 1000
db-plan-enabled = true
db-pool = 1
db-pool-acquisition-timeout = 10
db-pool-acquisition-timeout = 30
db-pool-max-lifetime = 3600
db-pre-request = "test.custom_headers"
db-prepared-statements = false
db-root-spec = "root"
Expand Down
3 changes: 2 additions & 1 deletion test/io/configs/expected/no-defaults.config
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ db-extra-search-path = "public,test"
db-max-rows = 1000
db-plan-enabled = true
db-pool = 1
db-pool-acquisition-timeout = 10
db-pool-acquisition-timeout = 30
db-pool-max-lifetime = 3600
db-pre-request = "please_run_fast"
db-prepared-statements = false
db-root-spec = "openapi_v3"
Expand Down
3 changes: 2 additions & 1 deletion test/io/configs/expected/types.config
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ db-extra-search-path = "public"
db-max-rows = ""
db-plan-enabled = false
db-pool = 10
db-pool-acquisition-timeout = ""
db-pool-acquisition-timeout = 10
db-pool-max-lifetime = 1800
db-pre-request = ""
db-prepared-statements = true
db-root-spec = ""
Expand Down
3 changes: 2 additions & 1 deletion test/io/configs/no-defaults-env.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ PGRST_DB_EXTRA_SEARCH_PATH: public, test
PGRST_DB_MAX_ROWS: 1000
PGRST_DB_PLAN_ENABLED: true
PGRST_DB_POOL: 1
PGRST_DB_POOL_ACQUISITION_TIMEOUT: 10
PGRST_DB_POOL_ACQUISITION_TIMEOUT: 30
PGRST_DB_POOL_MAX_LIFETIME: 3600
PGRST_DB_PREPARED_STATEMENTS: false
PGRST_DB_PRE_REQUEST: please_run_fast
PGRST_DB_ROOT_SPEC: openapi_v3
Expand Down
3 changes: 2 additions & 1 deletion test/io/configs/no-defaults.config
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ db-extra-search-path = "public, test"
db-max-rows = 1000
db-plan-enabled = true
db-pool = 1
db-pool-acquisition-timeout = 10
db-pool-acquisition-timeout = 30
db-pool-max-lifetime = 3600
db-pre-request = "please_run_fast"
db-prepared-statements = false
db-root-spec = "openapi_v3"
Expand Down
2 changes: 1 addition & 1 deletion test/spec/Main.hs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ import qualified Feature.RpcPreRequestGucsSpec

main :: IO ()
main = do
pool <- P.acquire 3 Nothing $ toUtf8 $ configDbUri testCfg
pool <- P.acquire 3 10 60 $ toUtf8 $ configDbUri testCfg

actualPgVersion <- either (panic . show) id <$> P.use pool queryPgVersion

Expand Down
3 changes: 2 additions & 1 deletion test/spec/SpecHelper.hs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ baseCfg = let secret = Just $ encodeUtf8 "reallyreallyreallyreallyverysafe" in
, configDbMaxRows = Nothing
, configDbPlanEnabled = False
, configDbPoolSize = 10
, configDbPoolAcquisitionTimeout = Nothing
, configDbPoolAcquisitionTimeout = 10
, configDbPoolMaxLifetime = 1800
, configDbPreRequest = Just $ QualifiedIdentifier "test" "switch_role"
, configDbPreparedStatements = True
, configDbRootSpec = Nothing
Expand Down

0 comments on commit f26cdd5

Please sign in to comment.