From 9555cc8d2b02c4191d67ba39f589b39b01735518 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Wed, 2 Oct 2019 09:55:27 +0900 Subject: [PATCH] Revert hooks for session start and end, take two The location of the session end hook has been chosen so as it is possible to allow modules to do their own transactions, however any trying to any any subsystem which went through before_shmem_exit() would cause issues, limiting the pluggability of the hook. Per discussion with Tom Lane and Andres Freund. Discussion: https://postgr.es/m/18722.1569906636@sss.pgh.pa.us --- src/backend/tcop/postgres.c | 6 - src/backend/utils/init/postinit.c | 6 - src/include/tcop/tcopprot.h | 7 - src/test/modules/Makefile | 1 - .../modules/test_session_hooks/.gitignore | 4 - src/test/modules/test_session_hooks/Makefile | 23 --- src/test/modules/test_session_hooks/README | 11 -- .../expected/test_session_hooks.out | 37 ----- .../test_session_hooks/session_hooks.conf | 2 - .../sql/test_session_hooks.sql | 19 --- .../test_session_hooks/test_session_hooks.c | 155 ------------------ 11 files changed, 271 deletions(-) delete mode 100644 src/test/modules/test_session_hooks/.gitignore delete mode 100644 src/test/modules/test_session_hooks/Makefile delete mode 100644 src/test/modules/test_session_hooks/README delete mode 100644 src/test/modules/test_session_hooks/expected/test_session_hooks.out delete mode 100644 src/test/modules/test_session_hooks/session_hooks.conf delete mode 100644 src/test/modules/test_session_hooks/sql/test_session_hooks.sql delete mode 100644 src/test/modules/test_session_hooks/test_session_hooks.c diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 6d80cc2d64..e8d8e6f828 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -171,9 +171,6 @@ static ProcSignalReason RecoveryConflictReason; static MemoryContext row_description_context = NULL; static StringInfoData row_description_buf; -/* Hook for plugins to get control at start of session */ -session_start_hook_type session_start_hook = NULL; - /* ---------------------------------------------------------------- * decls for routines only used in this file * ---------------------------------------------------------------- @@ -3971,9 +3968,6 @@ PostgresMain(int argc, char *argv[], if (!IsUnderPostmaster) PgStartTime = GetCurrentTimestamp(); - if (session_start_hook) - (*session_start_hook) (); - /* * POSTGRES main processing loop begins here * diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 151703a437..29c5ec7b58 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -78,8 +78,6 @@ static bool ThereIsAtLeastOneRole(void); static void process_startup_options(Port *port, bool am_superuser); static void process_settings(Oid databaseid, Oid roleid); -/* Hook for plugins to get control at end of session */ -session_end_hook_type session_end_hook = NULL; /*** InitPostgres support ***/ @@ -1197,10 +1195,6 @@ ShutdownPostgres(int code, Datum arg) * them explicitly. */ LockReleaseAll(USER_LOCKMETHOD, true); - - /* Hook at session end */ - if (session_end_hook) - (*session_end_hook) (); } diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.h index 63581048b2..ec21f7e45c 100644 --- a/src/include/tcop/tcopprot.h +++ b/src/include/tcop/tcopprot.h @@ -30,13 +30,6 @@ extern PGDLLIMPORT const char *debug_query_string; extern int max_stack_depth; extern int PostAuthDelay; -/* Hook for plugins to get control at start and end of session */ -typedef void (*session_start_hook_type) (void); -typedef void (*session_end_hook_type) (void); - -extern PGDLLIMPORT session_start_hook_type session_start_hook; -extern PGDLLIMPORT session_end_hook_type session_end_hook; - /* GUC-configurable parameters */ typedef enum diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile index abb3203903..b2eaef3bff 100644 --- a/src/test/modules/Makefile +++ b/src/test/modules/Makefile @@ -21,7 +21,6 @@ SUBDIRS = \ test_predtest \ test_rbtree \ test_rls_hooks \ - test_session_hooks \ test_shm_mq \ unsafe_tests \ worker_spi diff --git a/src/test/modules/test_session_hooks/.gitignore b/src/test/modules/test_session_hooks/.gitignore deleted file mode 100644 index 5dcb3ff972..0000000000 --- a/src/test/modules/test_session_hooks/.gitignore +++ /dev/null @@ -1,4 +0,0 @@ -# Generated subdirectories -/log/ -/results/ -/tmp_check/ diff --git a/src/test/modules/test_session_hooks/Makefile b/src/test/modules/test_session_hooks/Makefile deleted file mode 100644 index e8ba6990f6..0000000000 --- a/src/test/modules/test_session_hooks/Makefile +++ /dev/null @@ -1,23 +0,0 @@ -# src/test/modules/test_session_hooks/Makefile - -MODULE_big = test_session_hooks -OBJS = test_session_hooks.o $(WIN32RES) -PGFILEDESC = "test_session_hooks - tests for start and end session hooks" - -REGRESS = test_session_hooks -REGRESS_OPTS = --temp-config=$(top_srcdir)/src/test/modules/test_session_hooks/session_hooks.conf -# Disabled because these tests require extra configuration with -# "shared_preload_libraries=test_session_hooks", which typical -# installcheck users do not have (e.g. buildfarm clients). -NO_INSTALLCHECK = 1 - -ifdef USE_PGXS -PG_CONFIG = pg_config -PGXS := $(shell $(PG_CONFIG) --pgxs) -include $(PGXS) -else -subdir = src/test/modules/test_session_hooks -top_builddir = ../../../.. -include $(top_builddir)/src/Makefile.global -include $(top_srcdir)/contrib/contrib-global.mk -endif diff --git a/src/test/modules/test_session_hooks/README b/src/test/modules/test_session_hooks/README deleted file mode 100644 index 0fe3a986dc..0000000000 --- a/src/test/modules/test_session_hooks/README +++ /dev/null @@ -1,11 +0,0 @@ -test_session_hooks -================== - -test_session_hooks is an example of how to use session start and end -hooks. - -This module will insert into a pre-existing table called "session_hook_log" -a log activity which happens at session start and end. It is possible -to control which user information is logged when using the configuration -parameter "test_session_hooks.username". If set, the hooks will log only -information of the session user matching the parameter value. diff --git a/src/test/modules/test_session_hooks/expected/test_session_hooks.out b/src/test/modules/test_session_hooks/expected/test_session_hooks.out deleted file mode 100644 index 120dcc9a11..0000000000 --- a/src/test/modules/test_session_hooks/expected/test_session_hooks.out +++ /dev/null @@ -1,37 +0,0 @@ --- --- Tests for start and end session hooks --- --- Only activity from role regress_sess_hook_usr2 is logged. -CREATE ROLE regress_sess_hook_usr1 SUPERUSER LOGIN; -CREATE ROLE regress_sess_hook_usr2 SUPERUSER LOGIN; -\set prevdb :DBNAME -\set prevusr :USER -CREATE TABLE session_hook_log(id SERIAL, dbname TEXT, username TEXT, hook_at TEXT); -SELECT * FROM session_hook_log ORDER BY id; - id | dbname | username | hook_at -----+--------+----------+--------- -(0 rows) - -\c :prevdb regress_sess_hook_usr1 -SELECT * FROM session_hook_log ORDER BY id; - id | dbname | username | hook_at -----+--------+----------+--------- -(0 rows) - -\c :prevdb regress_sess_hook_usr2 -SELECT * FROM session_hook_log ORDER BY id; - id | dbname | username | hook_at -----+--------------------+------------------------+--------- - 1 | contrib_regression | regress_sess_hook_usr2 | START -(1 row) - -\c :prevdb :prevusr -SELECT * FROM session_hook_log ORDER BY id; - id | dbname | username | hook_at -----+--------------------+------------------------+--------- - 1 | contrib_regression | regress_sess_hook_usr2 | START - 2 | contrib_regression | regress_sess_hook_usr2 | END -(2 rows) - -DROP ROLE regress_sess_hook_usr1; -DROP ROLE regress_sess_hook_usr2; diff --git a/src/test/modules/test_session_hooks/session_hooks.conf b/src/test/modules/test_session_hooks/session_hooks.conf deleted file mode 100644 index fc62b4adef..0000000000 --- a/src/test/modules/test_session_hooks/session_hooks.conf +++ /dev/null @@ -1,2 +0,0 @@ -shared_preload_libraries = 'test_session_hooks' -test_session_hooks.username = regress_sess_hook_usr2 diff --git a/src/test/modules/test_session_hooks/sql/test_session_hooks.sql b/src/test/modules/test_session_hooks/sql/test_session_hooks.sql deleted file mode 100644 index 1cc06f613b..0000000000 --- a/src/test/modules/test_session_hooks/sql/test_session_hooks.sql +++ /dev/null @@ -1,19 +0,0 @@ --- --- Tests for start and end session hooks --- - --- Only activity from role regress_sess_hook_usr2 is logged. -CREATE ROLE regress_sess_hook_usr1 SUPERUSER LOGIN; -CREATE ROLE regress_sess_hook_usr2 SUPERUSER LOGIN; -\set prevdb :DBNAME -\set prevusr :USER -CREATE TABLE session_hook_log(id SERIAL, dbname TEXT, username TEXT, hook_at TEXT); -SELECT * FROM session_hook_log ORDER BY id; -\c :prevdb regress_sess_hook_usr1 -SELECT * FROM session_hook_log ORDER BY id; -\c :prevdb regress_sess_hook_usr2 -SELECT * FROM session_hook_log ORDER BY id; -\c :prevdb :prevusr -SELECT * FROM session_hook_log ORDER BY id; -DROP ROLE regress_sess_hook_usr1; -DROP ROLE regress_sess_hook_usr2; diff --git a/src/test/modules/test_session_hooks/test_session_hooks.c b/src/test/modules/test_session_hooks/test_session_hooks.c deleted file mode 100644 index a90c373c69..0000000000 --- a/src/test/modules/test_session_hooks/test_session_hooks.c +++ /dev/null @@ -1,155 +0,0 @@ -/* ------------------------------------------------------------------------- - * - * test_session_hooks.c - * Code for testing start and end session hooks. - * - * Portions Copyright (c) 1996-2019, PostgreSQL Global Development Group - * Portions Copyright (c) 1994, Regents of the University of California - * - * IDENTIFICATION - * src/test/modules/test_session_hooks/test_session_hooks.c - * - * ------------------------------------------------------------------------- - */ -#include "postgres.h" - -#include "access/parallel.h" -#include "access/xact.h" -#include "commands/dbcommands.h" -#include "executor/spi.h" -#include "lib/stringinfo.h" -#include "miscadmin.h" -#include "tcop/tcopprot.h" -#include "utils/snapmgr.h" -#include "utils/builtins.h" - -PG_MODULE_MAGIC; - -/* Entry point of library loading/unloading */ -void _PG_init(void); -void _PG_fini(void); - -/* GUC variables */ -static char *session_hook_username = "postgres"; - -/* Previous hooks on stack */ -static session_start_hook_type prev_session_start_hook = NULL; -static session_end_hook_type prev_session_end_hook = NULL; - -static void -register_session_hook(const char *hook_at) -{ - const char *username; - - StartTransactionCommand(); - SPI_connect(); - PushActiveSnapshot(GetTransactionSnapshot()); - - /* Check the current user validity */ - username = GetUserNameFromId(GetUserId(), false); - - /* Register log just for configured username */ - if (strcmp(username, session_hook_username) == 0) - { - const char *dbname; - int ret; - StringInfoData buf; - - dbname = get_database_name(MyDatabaseId); - - initStringInfo(&buf); - - appendStringInfo(&buf, "INSERT INTO session_hook_log (dbname, username, hook_at) "); - appendStringInfo(&buf, "VALUES (%s, %s, %s);", - quote_literal_cstr(dbname), - quote_literal_cstr(username), - quote_literal_cstr(hook_at)); - - ret = SPI_exec(buf.data, 0); - if (ret != SPI_OK_INSERT) - elog(ERROR, "SPI_execute failed: error code %d", ret); - } - - SPI_finish(); - PopActiveSnapshot(); - CommitTransactionCommand(); -} - -/* sample session start hook function */ -static void -sample_session_start_hook(void) -{ - if (prev_session_start_hook) - prev_session_start_hook(); - - /* consider only normal backends */ - if (MyBackendId == InvalidBackendId) - return; - - /* consider backends connected to a database */ - if (!OidIsValid(MyDatabaseId)) - return; - - /* no parallel workers */ - if (IsParallelWorker()) - return; - - register_session_hook("START"); -} - -/* sample session end hook function */ -static void -sample_session_end_hook(void) -{ - if (prev_session_end_hook) - prev_session_end_hook(); - - /* consider only normal backends */ - if (MyBackendId == InvalidBackendId) - return; - - /* consider backends connected to a database */ - if (!OidIsValid(MyDatabaseId)) - return; - - /* no parallel workers */ - if (IsParallelWorker()) - return; - - register_session_hook("END"); -} - -/* - * Module load callback - */ -void -_PG_init(void) -{ - /* Save previous hooks */ - prev_session_start_hook = session_start_hook; - prev_session_end_hook = session_end_hook; - - /* Set new hooks */ - session_start_hook = sample_session_start_hook; - session_end_hook = sample_session_end_hook; - - /* Load GUCs */ - DefineCustomStringVariable("test_session_hooks.username", - "Username to register log on session start or end", - NULL, - &session_hook_username, - "postgres", - PGC_SIGHUP, - 0, NULL, NULL, NULL); -} - -/* - * Module unload callback - */ -void -_PG_fini(void) -{ - /* Uninstall hooks */ - session_start_hook = prev_session_start_hook; - session_end_hook = prev_session_end_hook; -}