Skip to content

Commit

Permalink
Change bgw_log_level to use PGC_SUSET
Browse files Browse the repository at this point in the history
Right now `bgw_log_level` requires `ALTER SYSTEM` since it is using
`PGC_SIGHUP` but we want to make sure that it is possible to set the
scheduler log level using `ALTER DATABASE` which allows it to be
replicated and also allows the database owner to set the value rather
than requiring superuser privileges or explicit grants.

Since we want to allow configuration to be reloaded without restarting
the server, we are limited to `PGC_SUSET` and `PGC_SIGHUP` and tests
are added for PG15 and later to make sure that we can grant privileges
to use `ALTER SYSTEM` and `ALTER DATABASE` to set the parameter.
  • Loading branch information
mkindahl committed Dec 21, 2023
1 parent a1f7d35 commit df3864d
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 8 deletions.
1 change: 1 addition & 0 deletions .unreleased/fix_6384
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes: #6384 Change bgw_log_level to use PGC_SUSET
2 changes: 1 addition & 1 deletion src/guc.c
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ _guc_init(void)
/* valueAddr= */ &ts_guc_bgw_log_level,
/* bootValue= */ WARNING,
/* options= */ loglevel_options,
/* context= */ PGC_SIGHUP,
/* context= */ PGC_SUSET,
0,
NULL,
NULL,
Expand Down
19 changes: 16 additions & 3 deletions tsl/test/expected/bgw_scheduler_control.out
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ CREATE FUNCTION ts_bgw_params_destroy() RETURNS VOID
AS :MODULE_PATHNAME LANGUAGE C VOLATILE;
CREATE FUNCTION ts_bgw_params_reset_time(set_time BIGINT, wait BOOLEAN) RETURNS VOID
AS :MODULE_PATHNAME LANGUAGE C VOLATILE;
ALTER DATABASE :TEST_DBNAME OWNER TO :ROLE_DEFAULT_PERM_USER;
GRANT EXECUTE ON FUNCTION pg_reload_conf TO :ROLE_DEFAULT_PERM_USER;
GRANT ALTER SYSTEM, SET ON PARAMETER timescaledb.bgw_log_level TO :ROLE_DEFAULT_PERM_USER;
-- These are needed to set up the test scheduler
CREATE TABLE public.bgw_dsm_handle_store(handle BIGINT);
INSERT INTO public.bgw_dsm_handle_store VALUES (0);
Expand Down Expand Up @@ -39,14 +42,18 @@ TRUNCATE _timescaledb_internal.bgw_job_stat;
--
-- Debug messages should be in log now which it wasn't before.
--
\c :TEST_DBNAME :ROLE_SUPERUSER
ALTER SYSTEM SET timescaledb.bgw_log_level = 'DEBUG1';
-- We change user to make sure that granting SET and ALTER SYSTEM
-- privileges to the default user actually works.
--
SET ROLE :ROLE_DEFAULT_PERM_USER;
ALTER DATABASE :TEST_DBNAME SET timescaledb.bgw_log_level = 'DEBUG1';
SELECT pg_reload_conf();
pg_reload_conf
----------------
t
(1 row)

RESET ROLE;
SELECT ts_bgw_params_reset_time(0, false);
ts_bgw_params_reset_time
--------------------------
Expand Down Expand Up @@ -94,7 +101,7 @@ SELECT * FROM cleaned_bgw_log;
4 | DB Scheduler | database scheduler for database (RANDOM) exiting
(7 rows)

ALTER SYSTEM RESET timescaledb.bgw_log_level;
ALTER DATABASE :TEST_DBNAME RESET timescaledb.bgw_log_level;
SELECT pg_reload_conf();
pg_reload_conf
----------------
Expand Down Expand Up @@ -126,3 +133,9 @@ SELECT delete_job(:job_id);

(1 row)

SET ROLE :ROLE_DEFAULT_PERM_USER;
-- Make sure we can set the variable using ALTER SYSTEM using the
-- previous grants. We don't bother about checking that it has an
-- effect here since we already knows it works from the above code.
ALTER SYSTEM SET timescaledb.bgw_log_level TO 'DEBUG2';
ALTER SYSTEM RESET timescaledb.bgw_log_level;
4 changes: 3 additions & 1 deletion tsl/test/sql/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ if(CMAKE_BUILD_TYPE MATCHES Debug)
APPEND
TEST_FILES
bgw_db_scheduler.sql
bgw_scheduler_control.sql
job_errors_permissions.sql
troubleshooting_job_errors.sql
bgw_db_scheduler_fixed.sql
Expand Down Expand Up @@ -94,6 +93,9 @@ if((${PG_VERSION_MAJOR} GREATER_EQUAL "14"))
endif()

if((${PG_VERSION_MAJOR} GREATER_EQUAL "15"))
if(CMAKE_BUILD_TYPE MATCHES Debug)
list(APPEND TEST_FILES bgw_scheduler_control.sql)
endif()
list(APPEND TEST_FILES merge_compress.sql)
endif()

Expand Down
22 changes: 19 additions & 3 deletions tsl/test/sql/bgw_scheduler_control.sql
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ AS :MODULE_PATHNAME LANGUAGE C VOLATILE;
CREATE FUNCTION ts_bgw_params_reset_time(set_time BIGINT, wait BOOLEAN) RETURNS VOID
AS :MODULE_PATHNAME LANGUAGE C VOLATILE;

ALTER DATABASE :TEST_DBNAME OWNER TO :ROLE_DEFAULT_PERM_USER;
GRANT EXECUTE ON FUNCTION pg_reload_conf TO :ROLE_DEFAULT_PERM_USER;
GRANT ALTER SYSTEM, SET ON PARAMETER timescaledb.bgw_log_level TO :ROLE_DEFAULT_PERM_USER;

-- These are needed to set up the test scheduler
CREATE TABLE public.bgw_dsm_handle_store(handle BIGINT);
INSERT INTO public.bgw_dsm_handle_store VALUES (0);
Expand Down Expand Up @@ -43,10 +47,14 @@ TRUNCATE _timescaledb_internal.bgw_job_stat;
--
-- Debug messages should be in log now which it wasn't before.
--
\c :TEST_DBNAME :ROLE_SUPERUSER
ALTER SYSTEM SET timescaledb.bgw_log_level = 'DEBUG1';
-- We change user to make sure that granting SET and ALTER SYSTEM
-- privileges to the default user actually works.
--
SET ROLE :ROLE_DEFAULT_PERM_USER;
ALTER DATABASE :TEST_DBNAME SET timescaledb.bgw_log_level = 'DEBUG1';
SELECT pg_reload_conf();

RESET ROLE;
SELECT ts_bgw_params_reset_time(0, false);
INSERT INTO _timescaledb_config.bgw_job(
application_name,
Expand Down Expand Up @@ -75,7 +83,7 @@ INSERT INTO _timescaledb_config.bgw_job(
SELECT ts_bgw_db_scheduler_test_run_and_wait_for_scheduler_finish(25, 0);
SELECT * FROM cleaned_bgw_log;

ALTER SYSTEM RESET timescaledb.bgw_log_level;
ALTER DATABASE :TEST_DBNAME RESET timescaledb.bgw_log_level;
SELECT pg_reload_conf();

TRUNCATE bgw_log;
Expand All @@ -84,3 +92,11 @@ SELECT ts_bgw_db_scheduler_test_run_and_wait_for_scheduler_finish(25, 0);
SELECT * FROM cleaned_bgw_log;

SELECT delete_job(:job_id);

SET ROLE :ROLE_DEFAULT_PERM_USER;

-- Make sure we can set the variable using ALTER SYSTEM using the
-- previous grants. We don't bother about checking that it has an
-- effect here since we already knows it works from the above code.
ALTER SYSTEM SET timescaledb.bgw_log_level TO 'DEBUG2';
ALTER SYSTEM RESET timescaledb.bgw_log_level;

0 comments on commit df3864d

Please sign in to comment.