Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: enable in-database configuration for pool settings #3137

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 15 additions & 6 deletions src/PostgREST/AppState.hs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ data AuthResult = AuthResult

data AppState = AppState
-- | Database connection pool
{ statePool :: SQL.Pool
{ statePool :: IORef SQL.Pool
-- | Database server version, will be updated by the connectionWorker
, statePgVersion :: IORef PgVersion
-- | No schema cache at the start. Will be filled in by the connectionWorker
Expand Down Expand Up @@ -125,8 +125,9 @@ init conf = do

initWithPool :: AppSockets -> SQL.Pool -> AppConfig -> IO AppState
initWithPool (sock, adminSock) pool conf = do
appState <- AppState pool
<$> newIORef minimumPgVersion -- assume we're in a supported version when starting, this will be corrected on a later step
appState <- AppState
<$> newIORef pool
<*> newIORef minimumPgVersion -- assume we're in a supported version when starting, this will be corrected on a later step
<*> newIORef Nothing
<*> pure (pure ())
<*> newEmptyMVar
Expand Down Expand Up @@ -208,7 +209,8 @@ initPool AppConfig{..} =
-- | Run an action with a database connection.
usePool :: AppState -> AppConfig -> SQL.Session a -> IO (Either SQL.UsageError a)
usePool appState@AppState{..} AppConfig{configLogLevel} x = do
res <- SQL.use statePool x
pool <- getPool appState
res <- SQL.use pool x

when (configLogLevel > LogCrit) $ do
whenLeft res (\case
Expand All @@ -223,11 +225,17 @@ usePool appState@AppState{..} AppConfig{configLogLevel} x = do
-- | Flush the connection pool so that any future use of the pool will
-- use connections freshly established after this call.
flushPool :: AppState -> IO ()
flushPool AppState{..} = SQL.release statePool
flushPool appState = SQL.release =<< getPool appState

-- | Destroy the pool on shutdown.
destroyPool :: AppState -> IO ()
destroyPool AppState{..} = SQL.release statePool
destroyPool appState = SQL.release =<< getPool appState

getPool :: AppState -> IO SQL.Pool
getPool = readIORef . statePool

putPool :: AppState -> SQL.Pool -> IO ()
putPool = atomicWriteIORef . statePool

getPgVersion :: AppState -> IO PgVersion
getPgVersion = readIORef . statePgVersion
Expand Down Expand Up @@ -478,6 +486,7 @@ reReadConfig startingUp appState = do
logWithZTime appState $ "Failed reloading config: " <> err
Right newConf -> do
putConfig appState newConf
putPool appState =<< initPool newConf
if startingUp then
pass
else
Expand Down
4 changes: 4 additions & 0 deletions src/PostgREST/Config/Database.hs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ dbSettingsNames =
,"db_pre_config"
,"db_extra_search_path"
,"db_max_rows"
,"db_pool"
,"db_pool_acquisition_timeout"
,"db_pool_max_idletime"
,"db_pool_max_lifetime"
,"db_plan_enabled"
,"db_pre_request"
,"db_prepared_statements"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ db-channel-enabled = false
db-extra-search-path = "public,extensions,other"
db-max-rows = 100
db-plan-enabled = true
db-pool = 1
db-pool-acquisition-timeout = 30
db-pool-max-lifetime = 3600
db-pool-max-idletime = 60
db-pool = 3
db-pool-acquisition-timeout = 20
db-pool-max-lifetime = 360
db-pool-max-idletime = 20
db-pool-automatic-recovery = false
db-pre-request = "test.other_custom_headers"
db-prepared-statements = false
Expand Down
8 changes: 4 additions & 4 deletions test/io/configs/expected/no-defaults-with-db.config
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ db-channel-enabled = false
db-extra-search-path = "public,extensions,private"
db-max-rows = 1000
db-plan-enabled = true
db-pool = 1
db-pool-acquisition-timeout = 30
db-pool-max-lifetime = 3600
db-pool-max-idletime = 60
db-pool = 2
db-pool-acquisition-timeout = 15
db-pool-max-lifetime = 180
db-pool-max-idletime = 10
db-pool-automatic-recovery = false
db-pre-request = "test.custom_headers"
db-prepared-statements = false
Expand Down
12 changes: 8 additions & 4 deletions test/io/db_config.sql
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ ALTER ROLE db_config_authenticator SET pgrst.db_prepared_statements = 'false';
ALTER ROLE db_config_authenticator SET pgrst.db_pre_request = 'test.custom_headers';
ALTER ROLE db_config_authenticator SET pgrst.db_max_rows = '1000';
ALTER ROLE db_config_authenticator SET pgrst.db_extra_search_path = 'public, extensions';
ALTER ROLE db_config_authenticator SET pgrst.db_pool = '2';
ALTER ROLE db_config_authenticator SET pgrst.db_pool_acquisition_timeout = '15';
ALTER ROLE db_config_authenticator SET pgrst.db_pool_max_lifetime = '180';
ALTER ROLE db_config_authenticator SET pgrst.db_pool_max_idletime = '10';
ALTER ROLE db_config_authenticator SET pgrst.not_existing = 'should be ignored';
ALTER ROLE db_config_authenticator SET pgrst.server_cors_allowed_origins = 'http://example.com';
ALTER ROLE db_config_authenticator SET pgrst.server_trace_header = 'CF-Ray';
Expand All @@ -42,11 +46,7 @@ ALTER ROLE db_config_authenticator SET pgrst.log_level = 'ignored';
ALTER ROLE db_config_authenticator SET pgrst.db_uri = 'postgresql://ignored';
ALTER ROLE db_config_authenticator SET pgrst.db_channel_enabled = 'ignored';
ALTER ROLE db_config_authenticator SET pgrst.db_channel = 'ignored';
ALTER ROLE db_config_authenticator SET pgrst.db_pool = 'ignored';
ALTER ROLE db_config_authenticator SET pgrst.db_pool_timeout = 'ignored';
ALTER ROLE db_config_authenticator SET pgrst.db_pool_acquisition_timeout = 'ignored';
ALTER ROLE db_config_authenticator SET pgrst.db_pool_max_lifetime = 'ignored';
ALTER ROLE db_config_authenticator SET pgrst.db_pool_max_idletime = 'ignored';
ALTER ROLE db_config_authenticator SET pgrst.db_config = 'true';

-- other authenticator reloadable config options
Expand All @@ -64,6 +64,10 @@ ALTER ROLE other_authenticator SET pgrst.db_prepared_statements = 'false';
ALTER ROLE other_authenticator SET pgrst.db_pre_request = 'test.other_custom_headers';
ALTER ROLE other_authenticator SET pgrst.db_max_rows = '100';
ALTER ROLE other_authenticator SET pgrst.db_extra_search_path = 'public, extensions, other';
ALTER ROLE other_authenticator SET pgrst.db_pool = '3';
ALTER ROLE other_authenticator SET pgrst.db_pool_acquisition_timeout = '20';
ALTER ROLE other_authenticator SET pgrst.db_pool_max_lifetime = '360';
ALTER ROLE other_authenticator SET pgrst.db_pool_max_idletime = '20';
ALTER ROLE other_authenticator SET pgrst.openapi_mode = 'disabled';
ALTER ROLE other_authenticator SET pgrst.openapi_security_active = 'false';
ALTER ROLE other_authenticator SET pgrst.server_cors_allowed_origins = 'http://example.com';
Expand Down
8 changes: 8 additions & 0 deletions test/io/fixtures.sql
Original file line number Diff line number Diff line change
Expand Up @@ -198,3 +198,11 @@ $$ language sql set statement_timeout = '4s';
create function get_postgres_version() returns int as $$
select current_setting('server_version_num')::int;
$$ language sql;

create function change_db_pool_config(size int) returns void as $_$
begin
execute format($$
alter role postgrest_test_authenticator set pgrst.db_pool = %L;
$$, size);
perform pg_notify('pgrst', 'reload config');
end $_$ volatile security definer language plpgsql;
36 changes: 36 additions & 0 deletions test/io/test_io.py
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,42 @@ def test_pool_acquisition_timeout(level, defaultenv, metapostgrest):
assert "Timed out acquiring connection from connection pool." in output[1]


def test_db_pool_configuration_reload(defaultenv):
"Verify that PGRST_DB_POOL setting is reloaded correctly"

env = {
**defaultenv,
"PGRST_DB_POOL": "1",
"PGRST_DB_POOL_ACQUISITION_TIMEOUT": "1", # 1 second
}

with run(env=env) as postgrest:
response = postgrest.session.post(
"/rpc/change_db_pool_config", data={"size": 2}
)
assert response.status_code == 204

sleep_until_postgrest_config_reload()

def sleep():
res = postgrest.session.get("/rpc/sleep?seconds=1")
assert res.status_code == 204

def get_projects():
res = postgrest.session.get("/projects")
assert res.status_code == 200

thread_1 = Thread(target=sleep)
thread_1.start()

# This would return an error if the DB_POOL is only 1
thread_2 = Thread(target=get_projects)
thread_2.start()

thread_1.join()
thread_2.join()


def test_change_statement_timeout_held_connection(defaultenv, metapostgrest):
"Statement timeout changes take effect immediately, even with a request outliving the reconfiguration"

Expand Down