public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v46 6/7] Remove the GUC stats_temp_directory 2+ messages / 2 participants [nested] [flat]
* [PATCH v46 6/7] Remove the GUC stats_temp_directory @ 2020-09-29 13:59 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Kyotaro Horiguchi @ 2020-09-29 13:59 UTC (permalink / raw) The new stats collection system doesn't need temporary directory, so just remove it. pg_stat_statements modified to use pg_stat directory to store its temporary files. As the result basebackup copies the pg_stat_statments' temporary file if exists. --- .../pg_stat_statements/pg_stat_statements.c | 13 +++--- doc/src/sgml/backup.sgml | 2 - doc/src/sgml/config.sgml | 19 -------- doc/src/sgml/storage.sgml | 6 --- src/backend/postmaster/pgstat.c | 10 ----- src/backend/replication/basebackup.c | 36 ---------------- src/backend/utils/misc/guc.c | 43 ------------------- src/backend/utils/misc/postgresql.conf.sample | 1 - src/bin/initdb/initdb.c | 1 - src/bin/pg_rewind/filemap.c | 7 --- src/include/pgstat.h | 3 -- src/test/perl/PostgresNode.pm | 4 -- 12 files changed, 6 insertions(+), 139 deletions(-) diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index 72a117fc19..0a98b2f2c0 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -89,14 +89,13 @@ PG_MODULE_MAGIC; #define PGSS_DUMP_FILE PGSTAT_STAT_PERMANENT_DIRECTORY "/pg_stat_statements.stat" /* - * Location of external query text file. We don't keep it in the core - * system's stats_temp_directory. The core system can safely use that GUC - * setting, because the statistics collector temp file paths are set only once - * as part of changing the GUC, but pg_stat_statements has no way of avoiding - * race conditions. Besides, we only expect modest, infrequent I/O for query - * strings, so placing the file on a faster filesystem is not compelling. + * Location of external query text file. We don't keep it in the core system's + * pg_stats. pg_stat_statements has no way of avoiding race conditions even if + * the directory were specified by a GUC. Besides, we only expect modest, + * infrequent I/O for query strings, so placing the file on a faster filesystem + * is not compelling. */ -#define PGSS_TEXT_FILE PG_STAT_TMP_DIR "/pgss_query_texts.stat" +#define PGSS_TEXT_FILE PGSTAT_STAT_PERMANENT_DIRECTORY "/pgss_query_texts.stat" /* Magic number identifying the stats file format */ static const uint32 PGSS_FILE_HEADER = 0x20201218; diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml index 3c8aaed0b6..7557a375f0 100644 --- a/doc/src/sgml/backup.sgml +++ b/doc/src/sgml/backup.sgml @@ -1146,8 +1146,6 @@ SELECT pg_stop_backup(); <filename>pg_snapshots/</filename>, <filename>pg_stat_tmp/</filename>, and <filename>pg_subtrans/</filename> (but not the directories themselves) can be omitted from the backup as they will be initialized on postmaster startup. - If <xref linkend="guc-stats-temp-directory"/> is set and is under the data - directory then the contents of that directory can also be omitted. </para> <para> diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index b0c25c9c5c..084bc57779 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -7440,25 +7440,6 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; </listitem> </varlistentry> - <varlistentry id="guc-stats-temp-directory" xreflabel="stats_temp_directory"> - <term><varname>stats_temp_directory</varname> (<type>string</type>) - <indexterm> - <primary><varname>stats_temp_directory</varname> configuration parameter</primary> - </indexterm> - </term> - <listitem> - <para> - Sets the directory to store temporary statistics data in. This can be - a path relative to the data directory or an absolute path. The default - is <filename>pg_stat_tmp</filename>. Pointing this at a RAM-based - file system will decrease physical I/O requirements and can lead to - improved performance. - This parameter can only be set in the <filename>postgresql.conf</filename> - file or on the server command line. - </para> - </listitem> - </varlistentry> - </variablelist> </sect2> diff --git a/doc/src/sgml/storage.sgml b/doc/src/sgml/storage.sgml index 3234adb639..6bac5e075e 100644 --- a/doc/src/sgml/storage.sgml +++ b/doc/src/sgml/storage.sgml @@ -120,12 +120,6 @@ Item subsystem</entry> </row> -<row> - <entry><filename>pg_stat_tmp</filename></entry> - <entry>Subdirectory containing temporary files for the statistics - subsystem</entry> -</row> - <row> <entry><filename>pg_subtrans</filename></entry> <entry>Subdirectory containing subtransaction status data</entry> diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c index ecf9d9adcc..73b44a2652 100644 --- a/src/backend/postmaster/pgstat.c +++ b/src/backend/postmaster/pgstat.c @@ -99,16 +99,6 @@ bool pgstat_track_counts = false; int pgstat_track_functions = TRACK_FUNC_OFF; int pgstat_track_activity_query_size = 1024; -/* ---------- - * Built from GUC parameter - * ---------- - */ -char *pgstat_stat_directory = NULL; - -/* No longer used, but will be removed with GUC */ -char *pgstat_stat_filename = NULL; -char *pgstat_stat_tmpname = NULL; - /* * WAL usage counters saved from pgWALUsage at the previous call to * pgstat_send_wal(). This is used to calculate how much WAL usage diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c index d21801cf90..d2c3064678 100644 --- a/src/backend/replication/basebackup.c +++ b/src/backend/replication/basebackup.c @@ -87,9 +87,6 @@ static int basebackup_read_file(int fd, char *buf, size_t nbytes, off_t offset, /* Was the backup currently in-progress initiated in recovery mode? */ static bool backup_started_in_recovery = false; -/* Relative path of temporary statistics directory */ -static char *statrelpath = NULL; - /* * Size of each block sent into the tar stream for larger files. */ @@ -152,13 +149,6 @@ struct exclude_list_item */ static const char *const excludeDirContents[] = { - /* - * Skip temporary statistics files. PG_STAT_TMP_DIR must be skipped even - * when stats_temp_directory is set because PGSS_TEXT_FILE is always - * created there. - */ - PG_STAT_TMP_DIR, - /* * It is generally not useful to backup the contents of this directory * even if the intention is to restore to another primary. See backup.sgml @@ -261,7 +251,6 @@ perform_base_backup(basebackup_options *opt) StringInfo labelfile; StringInfo tblspc_map_file; backup_manifest_info manifest; - int datadirpathlen; List *tablespaces = NIL; backup_total = 0; @@ -284,8 +273,6 @@ perform_base_backup(basebackup_options *opt) Assert(CurrentResourceOwner == NULL); CurrentResourceOwner = ResourceOwnerCreate(NULL, "base backup"); - datadirpathlen = strlen(DataDir); - backup_started_in_recovery = RecoveryInProgress(); labelfile = makeStringInfo(); @@ -314,18 +301,6 @@ perform_base_backup(basebackup_options *opt) tablespaceinfo *ti; int tblspc_streamed = 0; - /* - * Calculate the relative path of temporary statistics directory in - * order to skip the files which are located in that directory later. - */ - if (is_absolute_path(pgstat_stat_directory) && - strncmp(pgstat_stat_directory, DataDir, datadirpathlen) == 0) - statrelpath = psprintf("./%s", pgstat_stat_directory + datadirpathlen + 1); - else if (strncmp(pgstat_stat_directory, "./", 2) != 0) - statrelpath = psprintf("./%s", pgstat_stat_directory); - else - statrelpath = pgstat_stat_directory; - /* Add a node for the base directory at the end */ ti = palloc0(sizeof(tablespaceinfo)); ti->size = -1; @@ -1377,17 +1352,6 @@ sendDir(const char *path, int basepathlen, bool sizeonly, List *tablespaces, if (excludeFound) continue; - /* - * Exclude contents of directory specified by statrelpath if not set - * to the default (pg_stat_tmp) which is caught in the loop above. - */ - if (statrelpath != NULL && strcmp(pathbuf, statrelpath) == 0) - { - elog(DEBUG1, "contents of directory \"%s\" excluded from backup", statrelpath); - size += _tarWriteDir(pathbuf, basepathlen, &statbuf, sizeonly); - continue; - } - /* * We can skip pg_wal, the WAL segments need to be fetched from the * WAL archive anyway. But include it as an empty directory anyway, so diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 85299e2138..16e430fb28 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -203,7 +203,6 @@ static bool check_autovacuum_work_mem(int *newval, void **extra, GucSource sourc static bool check_effective_io_concurrency(int *newval, void **extra, GucSource source); static bool check_maintenance_io_concurrency(int *newval, void **extra, GucSource source); static bool check_huge_page_size(int *newval, void **extra, GucSource source); -static void assign_pgstat_temp_directory(const char *newval, void *extra); static bool check_application_name(char **newval, void **extra, GucSource source); static void assign_application_name(const char *newval, void *extra); static bool check_cluster_name(char **newval, void **extra, GucSource source); @@ -560,8 +559,6 @@ char *HbaFileName; char *IdentFileName; char *external_pid_file; -char *pgstat_temp_directory; - char *application_name; int tcp_keepalives_idle; @@ -4355,17 +4352,6 @@ static struct config_string ConfigureNamesString[] = NULL, NULL, NULL }, - { - {"stats_temp_directory", PGC_SIGHUP, STATS_ACTIVITY, - gettext_noop("Writes temporary statistics files to the specified directory."), - NULL, - GUC_SUPERUSER_ONLY - }, - &pgstat_temp_directory, - PG_STAT_TMP_DIR, - check_canonical_path, assign_pgstat_temp_directory, NULL - }, - { {"synchronous_standby_names", PGC_SIGHUP, REPLICATION_PRIMARY, gettext_noop("Number of synchronous standbys and list of names of potential synchronous ones."), @@ -11773,35 +11759,6 @@ check_huge_page_size(int *newval, void **extra, GucSource source) return true; } -static void -assign_pgstat_temp_directory(const char *newval, void *extra) -{ - /* check_canonical_path already canonicalized newval for us */ - char *dname; - char *tname; - char *fname; - - /* directory */ - dname = guc_malloc(ERROR, strlen(newval) + 1); /* runtime dir */ - sprintf(dname, "%s", newval); - - /* global stats */ - tname = guc_malloc(ERROR, strlen(newval) + 12); /* /global.tmp */ - sprintf(tname, "%s/global.tmp", newval); - fname = guc_malloc(ERROR, strlen(newval) + 13); /* /global.stat */ - sprintf(fname, "%s/global.stat", newval); - - if (pgstat_stat_directory) - free(pgstat_stat_directory); - pgstat_stat_directory = dname; - if (pgstat_stat_tmpname) - free(pgstat_stat_tmpname); - pgstat_stat_tmpname = tname; - if (pgstat_stat_filename) - free(pgstat_stat_filename); - pgstat_stat_filename = fname; -} - static bool check_application_name(char **newval, void **extra, GucSource source) { diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 4f5b6bdb12..20c24a9d78 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -587,7 +587,6 @@ #track_io_timing = off #track_functions = none # none, pl, all #track_activity_query_size = 1024 # (change requires restart) -#stats_temp_directory = 'pg_stat_tmp' # - Monitoring - diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index c854221a30..0f42e78d19 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -217,7 +217,6 @@ static const char *const subdirs[] = { "pg_replslot", "pg_tblspc", "pg_stat", - "pg_stat_tmp", "pg_xact", "pg_logical", "pg_logical/snapshots", diff --git a/src/bin/pg_rewind/filemap.c b/src/bin/pg_rewind/filemap.c index 2618b4c957..ab5cb51de7 100644 --- a/src/bin/pg_rewind/filemap.c +++ b/src/bin/pg_rewind/filemap.c @@ -87,13 +87,6 @@ struct exclude_list_item */ static const char *excludeDirContents[] = { - /* - * Skip temporary statistics files. PG_STAT_TMP_DIR must be skipped even - * when stats_temp_directory is set because PGSS_TEXT_FILE is always - * created there. - */ - "pg_stat_tmp", /* defined as PG_STAT_TMP_DIR */ - /* * It is generally not useful to backup the contents of this directory * even if the intention is to restore to another primary. See backup.sgml diff --git a/src/include/pgstat.h b/src/include/pgstat.h index 0472b728bf..d7c50eb4f9 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -33,9 +33,6 @@ #define PGSTAT_STAT_PERMANENT_FILENAME "pg_stat/saved_stats" #define PGSTAT_STAT_PERMANENT_TMPFILE "pg_stat/saved_stats.tmp" -/* Default directory to store temporary statistics data in */ -#define PG_STAT_TMP_DIR "pg_stat_tmp" - /* Values for track_functions GUC variable --- order is significant! */ typedef enum TrackFunctionsLevel { diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index 9667f7667e..dd41a43b4e 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -455,10 +455,6 @@ sub init print $conf TestLib::slurp_file($ENV{TEMP_CONFIG}) if defined $ENV{TEMP_CONFIG}; - # XXX Neutralize any stats_temp_directory in TEMP_CONFIG. Nodes running - # concurrently must not share a stats_temp_directory. - print $conf "stats_temp_directory = 'pg_stat_tmp'\n"; - if ($params{allows_streaming}) { if ($params{allows_streaming} eq "logical") -- 2.27.0 ----Next_Part(Thu_Jan_14_15_14_25_2021_903)-- Content-Type: Text/X-Patch; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="v46-0007-Exclude-pg_stat-directory-from-base-backup.patch" ^ permalink raw reply [nested|flat] 2+ messages in thread
* Support event trigger for logoff @ 2024-04-19 15:46 Japin Li <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Japin Li @ 2024-04-19 15:46 UTC (permalink / raw) To: PostgreSQL Hackers <[email protected]>; +Cc: Song JinZhou <[email protected]> Hi, hackers I see [1] has already implemented on login event trigger, why not implement the logoff event trigger? My friend Song Jinzhou and I try to implement the logoff event trigger, so attach it. Here is a problem with the regression test when using \c to create a new session, because it might be running concurrently, which may lead to the checking being unstable. Any thoughts? [1] https://www.postgresql.org/message-id/0d46d29f-4558-3af9-9c85-7774e14a7709%40postgrespro.ru -- Regards, Japin Li Attachments: [text/x-diff] v1-0001-Add-support-event-triggers-for-logoff.patch (25.2K, ../../ME3P282MB316614DBFF1CB7F7EB4C6C0AB60D2@ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM/2-v1-0001-Add-support-event-triggers-for-logoff.patch) download | inline diff: From ef7c6aa408d0a6a97d7b0d2e093b71e279b1b0dc Mon Sep 17 00:00:00 2001 From: Japin Li <[email protected]> Date: Mon, 15 Apr 2024 09:11:41 +0800 Subject: [PATCH v1 1/1] Add support event triggers for logoff --- doc/src/sgml/catalogs.sgml | 13 ++ doc/src/sgml/ecpg.sgml | 2 + doc/src/sgml/event-trigger.sgml | 5 + src/backend/commands/dbcommands.c | 17 +- src/backend/commands/event_trigger.c | 181 +++++++++++++++++- src/backend/tcop/postgres.c | 8 + src/backend/utils/cache/evtcache.c | 2 + src/backend/utils/init/globals.c | 1 + src/backend/utils/init/postinit.c | 1 + src/bin/psql/tab-complete.c | 2 +- src/include/catalog/pg_database.dat | 2 +- src/include/catalog/pg_database.h | 3 + src/include/commands/event_trigger.h | 1 + src/include/miscadmin.h | 1 + src/include/tcop/cmdtaglist.h | 1 + src/include/utils/evtcache.h | 1 + .../regress/expected/event_trigger_logoff.out | 49 +++++ src/test/regress/parallel_schedule | 2 + src/test/regress/sql/event_trigger_logoff.sql | 26 +++ 19 files changed, 309 insertions(+), 9 deletions(-) create mode 100644 src/test/regress/expected/event_trigger_logoff.out create mode 100644 src/test/regress/sql/event_trigger_logoff.sql diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 2907079e2a..621fbfde98 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -3076,6 +3076,19 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l </para></entry> </row> + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>dathaslogoffevt</structfield> <type>bool</type> + </para> + <para> + Indicates that there are logoff event triggers defined for this database. + This flag is used to avoid extra lookups on the + <structname>pg_event_trigger</structname> table during each backend + startup. This flag is used internally by <productname>PostgreSQL</productname> + and should not be manually altered or read for monitoring purposes. + </para></entry> + </row> + <row> <entry role="catalog_table_entry"><para role="column_definition"> <structfield>datconnlimit</structfield> <type>int4</type> diff --git a/doc/src/sgml/ecpg.sgml b/doc/src/sgml/ecpg.sgml index e7a53f3c9d..d223843157 100644 --- a/doc/src/sgml/ecpg.sgml +++ b/doc/src/sgml/ecpg.sgml @@ -4765,6 +4765,7 @@ encoding = 0 (type: 5) datistemplate = t (type: 1) datallowconn = t (type: 1) dathasloginevt = f (type: 1) +dathaslogoffevt = f (type: 1) datconnlimit = -1 (type: 5) datfrozenxid = 379 (type: 1) dattablespace = 1663 (type: 1) @@ -4790,6 +4791,7 @@ encoding = 0 (type: 5) datistemplate = f (type: 1) datallowconn = t (type: 1) dathasloginevt = f (type: 1) +dathaslogoffevt = f (type: 1) datconnlimit = -1 (type: 5) datfrozenxid = 379 (type: 1) dattablespace = 1663 (type: 1) diff --git a/doc/src/sgml/event-trigger.sgml b/doc/src/sgml/event-trigger.sgml index 8e009cca05..ed996a456e 100644 --- a/doc/src/sgml/event-trigger.sgml +++ b/doc/src/sgml/event-trigger.sgml @@ -29,6 +29,7 @@ occurs in the database in which it is defined. Currently, the only supported events are <literal>login</literal>, + <literal>logoff</literal>, <literal>ddl_command_start</literal>, <literal>ddl_command_end</literal>, <literal>table_rewrite</literal> @@ -54,6 +55,10 @@ the in-progress <literal>login</literal> trigger. </para> + <para> + The <literal>logoff</literal> event occurs when a user logs off the system. + </para> + <para> The <literal>ddl_command_start</literal> event occurs just before the execution of a <literal>CREATE</literal>, <literal>ALTER</literal>, <literal>DROP</literal>, diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index 8229dfa1f2..a6784a0ea6 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -116,7 +116,7 @@ static void movedb_failure_callback(int code, Datum arg); static bool get_db_info(const char *name, LOCKMODE lockmode, Oid *dbIdP, Oid *ownerIdP, int *encodingP, bool *dbIsTemplateP, bool *dbAllowConnP, bool *dbHasLoginEvtP, - TransactionId *dbFrozenXidP, MultiXactId *dbMinMultiP, + bool *dbHasLogoffEvtP, TransactionId *dbFrozenXidP, MultiXactId *dbMinMultiP, Oid *dbTablespace, char **dbCollate, char **dbCtype, char **dbLocale, char **dbIcurules, char *dbLocProvider, @@ -680,6 +680,7 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt) char *src_collversion = NULL; bool src_istemplate; bool src_hasloginevt = false; + bool src_haslogoffevt = false; bool src_allowconn; TransactionId src_frozenxid = InvalidTransactionId; MultiXactId src_minmxid = InvalidMultiXactId; @@ -981,7 +982,7 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt) if (!get_db_info(dbtemplate, ShareLock, &src_dboid, &src_owner, &src_encoding, &src_istemplate, &src_allowconn, &src_hasloginevt, - &src_frozenxid, &src_minmxid, &src_deftablespace, + &src_haslogoffevt, &src_frozenxid, &src_minmxid, &src_deftablespace, &src_collate, &src_ctype, &src_locale, &src_icurules, &src_locprovider, &src_collversion)) ereport(ERROR, @@ -1425,6 +1426,7 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt) new_record[Anum_pg_database_datistemplate - 1] = BoolGetDatum(dbistemplate); new_record[Anum_pg_database_datallowconn - 1] = BoolGetDatum(dballowconnections); new_record[Anum_pg_database_dathasloginevt - 1] = BoolGetDatum(src_hasloginevt); + new_record[Anum_pg_database_dathaslogoffevt - 1] = BoolGetDatum(src_haslogoffevt); new_record[Anum_pg_database_datconnlimit - 1] = Int32GetDatum(dbconnlimit); new_record[Anum_pg_database_datfrozenxid - 1] = TransactionIdGetDatum(src_frozenxid); new_record[Anum_pg_database_datminmxid - 1] = TransactionIdGetDatum(src_minmxid); @@ -1653,7 +1655,7 @@ dropdb(const char *dbname, bool missing_ok, bool force) pgdbrel = table_open(DatabaseRelationId, RowExclusiveLock); if (!get_db_info(dbname, AccessExclusiveLock, &db_id, NULL, NULL, - &db_istemplate, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL)) + &db_istemplate, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL)) { if (!missing_ok) { @@ -1868,7 +1870,7 @@ RenameDatabase(const char *oldname, const char *newname) rel = table_open(DatabaseRelationId, RowExclusiveLock); if (!get_db_info(oldname, AccessExclusiveLock, &db_id, NULL, NULL, NULL, - NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL)) + NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL)) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_DATABASE), errmsg("database \"%s\" does not exist", oldname))); @@ -1978,7 +1980,7 @@ movedb(const char *dbname, const char *tblspcname) pgdbrel = table_open(DatabaseRelationId, RowExclusiveLock); if (!get_db_info(dbname, AccessExclusiveLock, &db_id, NULL, NULL, NULL, - NULL, NULL, NULL, NULL, &src_tblspcoid, NULL, NULL, NULL, NULL, NULL, NULL)) + NULL, NULL, NULL, NULL, NULL, &src_tblspcoid, NULL, NULL, NULL, NULL, NULL, NULL)) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_DATABASE), errmsg("database \"%s\" does not exist", dbname))); @@ -2760,7 +2762,7 @@ static bool get_db_info(const char *name, LOCKMODE lockmode, Oid *dbIdP, Oid *ownerIdP, int *encodingP, bool *dbIsTemplateP, bool *dbAllowConnP, bool *dbHasLoginEvtP, - TransactionId *dbFrozenXidP, MultiXactId *dbMinMultiP, + bool *dbHasLogoffEvtP, TransactionId *dbFrozenXidP, MultiXactId *dbMinMultiP, Oid *dbTablespace, char **dbCollate, char **dbCtype, char **dbLocale, char **dbIcurules, char *dbLocProvider, @@ -2847,6 +2849,9 @@ get_db_info(const char *name, LOCKMODE lockmode, /* Has on login event trigger? */ if (dbHasLoginEvtP) *dbHasLoginEvtP = dbform->dathasloginevt; + /* Has on logoff event trigger? */ + if (dbHasLogoffEvtP) + *dbHasLogoffEvtP = dbform->dathaslogoffevt; /* allowing connections? */ if (dbAllowConnP) *dbAllowConnP = dbform->datallowconn; diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c index 7a5ed6b985..1adbd4ea12 100644 --- a/src/backend/commands/event_trigger.c +++ b/src/backend/commands/event_trigger.c @@ -112,6 +112,7 @@ static void EventTriggerInvoke(List *fn_oid_list, EventTriggerData *trigdata); static const char *stringify_grant_objtype(ObjectType objtype); static const char *stringify_adefprivs_objtype(ObjectType objtype); static void SetDatabaseHasLoginEventTriggers(void); +static void SetDatabaseHasLogoffEventTriggers(void); /* * Create an event trigger. @@ -143,6 +144,7 @@ CreateEventTrigger(CreateEventTrigStmt *stmt) strcmp(stmt->eventname, "ddl_command_end") != 0 && strcmp(stmt->eventname, "sql_drop") != 0 && strcmp(stmt->eventname, "login") != 0 && + strcmp(stmt->eventname, "logoff") != 0 && strcmp(stmt->eventname, "table_rewrite") != 0) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -179,6 +181,10 @@ CreateEventTrigger(CreateEventTrigStmt *stmt) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("tag filtering is not supported for login event triggers"))); + else if (strcmp(stmt->eventname, "logoff") == 0 && tags != NULL) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("tag filtering is not supported for logoff event triggers"))); /* * Give user a nice error message if an event trigger of the same name @@ -317,6 +323,13 @@ insert_event_trigger_tuple(const char *trigname, const char *eventname, Oid evtO if (strcmp(eventname, "login") == 0) SetDatabaseHasLoginEventTriggers(); + /* + * Logoff event triggers have an additional flag in pg_database to enable + * faster lookups in hot codepaths. Set the flag unless already True. + */ + if (strcmp(eventname, "logoff") == 0) + SetDatabaseHasLogoffEventTriggers(); + /* Depend on owner. */ recordDependencyOnOwner(EventTriggerRelationId, trigoid, evtOwner); @@ -413,6 +426,44 @@ SetDatabaseHasLoginEventTriggers(void) heap_freetuple(tuple); } +/* + * Set pg_database.dathaslogoffevt flag for current database indicating that + * current database has on logoff event triggers. + */ +void +SetDatabaseHasLogoffEventTriggers(void) +{ + /* Set dathaslogoffevt flag in pg_database */ + Form_pg_database db; + Relation pg_db = table_open(DatabaseRelationId, RowExclusiveLock); + HeapTuple tuple; + + /* + * Use shared lock to prevent a conflict with EventTriggerOnLogoff() trying + * to reset pg_database.dathaslogoffevt flag. Note, this lock doesn't + * effectively blocks database or other objection. It's just custom lock + * tag used to prevent multiple backends changing + * pg_database.dathaslogoffevt flag. + */ + LockSharedObject(DatabaseRelationId, MyDatabaseId, 0, AccessExclusiveLock); + + tuple = SearchSysCacheCopy1(DATABASEOID, ObjectIdGetDatum(MyDatabaseId)); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failed for database %u", MyDatabaseId); + db = (Form_pg_database) GETSTRUCT(tuple); + if (!db->dathaslogoffevt) + { + db->dathaslogoffevt = true; + CatalogTupleUpdate(pg_db, &tuple->t_self, tuple); + CommandCounterIncrement(); + + /* take effect for the current session */ + MyDatabaseHasLogoffEventTriggers = true; + } + table_close(pg_db, RowExclusiveLock); + heap_freetuple(tuple); +} + /* * ALTER EVENT TRIGGER foo ENABLE|DISABLE|ENABLE ALWAYS|REPLICA */ @@ -455,6 +506,14 @@ AlterEventTrigger(AlterEventTrigStmt *stmt) tgenabled != TRIGGER_DISABLED) SetDatabaseHasLoginEventTriggers(); + /* + * Logoff event triggers have an additional flag in pg_database to enable + * faster lookups in hot codepaths. Set the flag unless already True. + */ + if (namestrcmp(&evtForm->evtevent, "logoff") == 0 && + tgenabled != TRIGGER_DISABLED) + SetDatabaseHasLogoffEventTriggers(); + InvokeObjectPostAlterHook(EventTriggerRelationId, trigoid, 0); @@ -618,6 +677,8 @@ EventTriggerGetTag(Node *parsetree, EventTriggerEvent event) { if (event == EVT_Login) return CMDTAG_LOGIN; + else if (event == EVT_Logoff) + return CMDTAG_LOGOFF; else return CreateCommandTag(parsetree); } @@ -660,7 +721,8 @@ EventTriggerCommonSetup(Node *parsetree, if (event == EVT_DDLCommandStart || event == EVT_DDLCommandEnd || event == EVT_SQLDrop || - event == EVT_Login) + event == EVT_Login || + event == EVT_Logoff) { if (!command_tag_event_trigger_ok(dbgtag)) elog(ERROR, "unexpected command tag \"%s\"", GetCommandTagName(dbgtag)); @@ -998,6 +1060,123 @@ EventTriggerOnLogin(void) CommitTransactionCommand(); } +/* + * Fire logoff event triggers if any are present. The dathaslogoffevt + * pg_database flag is left unchanged when an event trigger is dropped to avoid + * complicating the codepath in the case of multiple event triggers. This + * function will instead unset the flag if no trigger is defined. + */ +void +EventTriggerOnLogoff(int code, Datum arg) +{ + List *runlist; + EventTriggerData trigdata; + + /* + * See EventTriggerDDLCommandStart for a discussion about why event + * triggers are disabled in single user mode or via a GUC. We also need a + * database connection (some background workers don't have it). + */ + if (!IsUnderPostmaster || !event_triggers || + !OidIsValid(MyDatabaseId) || !MyDatabaseHasLogoffEventTriggers) + return; + + StartTransactionCommand(); + runlist = EventTriggerCommonSetup(NULL, + EVT_Logoff, "logoff", + &trigdata, false); + + if (runlist != NIL) + { + /* + * Event trigger execution may require an active snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + + /* Run the triggers. */ + EventTriggerInvoke(runlist, &trigdata); + + /* Cleanup. */ + list_free(runlist); + + PopActiveSnapshot(); + } + + /* + * There is no active logoff event trigger, but our + * pg_database.dathaslogoffevt is set. Try to unset this flag. We use the + * lock to prevent concurrent SetDatabaseHasLogoffEventTriggers(), but we + * don't want to hang the connection waiting on the lock. Thus, we are + * just trying to acquire the lock conditionally. + */ + else if (ConditionalLockSharedObject(DatabaseRelationId, MyDatabaseId, + 0, AccessExclusiveLock)) + { + /* + * The lock is held. Now we need to recheck that logoff event triggers + * list is still empty. Once the list is empty, we know that even if + * there is a backend which concurrently inserts/enables a logoff event + * trigger, it will update pg_database.dathaslogoffevt *afterwards*. + */ + runlist = EventTriggerCommonSetup(NULL, + EVT_Logoff, "logoff", + &trigdata, true); + + if (runlist == NIL) + { + Relation pg_db = table_open(DatabaseRelationId, RowExclusiveLock); + HeapTuple tuple; + Form_pg_database db; + ScanKeyData key[1]; + SysScanDesc scan; + + /* + * Get the pg_database tuple to scribble on. Note that this does + * not directly rely on the syscache to avoid issues with + * flattened toast values for the in-place update. + */ + ScanKeyInit(&key[0], + Anum_pg_database_oid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(MyDatabaseId)); + + scan = systable_beginscan(pg_db, DatabaseOidIndexId, true, + NULL, 1, key); + tuple = systable_getnext(scan); + tuple = heap_copytuple(tuple); + systable_endscan(scan); + + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "could not find tuple for database %u", MyDatabaseId); + + db = (Form_pg_database) GETSTRUCT(tuple); + if (db->dathaslogoffevt) + { + db->dathaslogoffevt = false; + + /* + * Do an "in place" update of the pg_database tuple. Doing + * this instead of regular updates serves two purposes. First, + * that avoids possible waiting on the row-level lock. Second, + * that avoids dealing with TOAST. + * + * It's known that changes made by heap_inplace_update() may + * be lost due to concurrent normal updates. However, we are + * OK with that. The subsequent connections will still have a + * chance to set "dathaslogoffevt" to false. + */ + heap_inplace_update(pg_db, tuple); + } + table_close(pg_db, RowExclusiveLock); + heap_freetuple(tuple); + } + else + { + list_free(runlist); + } + } + CommitTransactionCommand(); +} /* * Fire table_rewrite triggers. diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 76f48b13d2..0b12a1da4b 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -4314,6 +4314,14 @@ PostgresMain(const char *dbname, const char *username) /* Fire any defined login event triggers, if appropriate */ EventTriggerOnLogin(); + /* + * Register a callback to fire any defined logoff event triggers, if + * appropriate. + */ + if (IsUnderPostmaster) + before_shmem_exit(EventTriggerOnLogoff, 0); + + /* * POSTGRES main processing loop begins here * diff --git a/src/backend/utils/cache/evtcache.c b/src/backend/utils/cache/evtcache.c index 185b52e669..803175bd03 100644 --- a/src/backend/utils/cache/evtcache.c +++ b/src/backend/utils/cache/evtcache.c @@ -168,6 +168,8 @@ BuildEventTriggerCache(void) event = EVT_TableRewrite; else if (strcmp(evtevent, "login") == 0) event = EVT_Login; + else if (strcmp(evtevent, "logoff") == 0) + event = EVT_Logoff; else continue; diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c index cc61937eef..79f45b6bbf 100644 --- a/src/backend/utils/init/globals.c +++ b/src/backend/utils/init/globals.c @@ -93,6 +93,7 @@ Oid MyDatabaseId = InvalidOid; Oid MyDatabaseTableSpace = InvalidOid; bool MyDatabaseHasLoginEventTriggers = false; +bool MyDatabaseHasLogoffEventTriggers = false; /* * DatabasePath is the path (relative to DataDir) of my database's diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 0805398e24..fc2944e027 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -1114,6 +1114,7 @@ InitPostgres(const char *in_dbname, Oid dboid, MyDatabaseTableSpace = datform->dattablespace; MyDatabaseHasLoginEventTriggers = datform->dathasloginevt; + MyDatabaseHasLogoffEventTriggers = datform->dathaslogoffevt; /* pass the database name back to the caller */ if (out_dbname) strcpy(out_dbname, dbname); diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 6fee3160f0..03a114dce4 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -3623,7 +3623,7 @@ psql_completion(const char *text, int start, int end) /* Complete CREATE EVENT TRIGGER <name> ON with event_type */ else if (Matches("CREATE", "EVENT", "TRIGGER", MatchAny, "ON")) COMPLETE_WITH("ddl_command_start", "ddl_command_end", "login", - "sql_drop", "table_rewrite"); + "logoff", "sql_drop", "table_rewrite"); /* * Complete CREATE EVENT TRIGGER <name> ON <event_type>. EXECUTE FUNCTION diff --git a/src/include/catalog/pg_database.dat b/src/include/catalog/pg_database.dat index c2ba636f8d..3402b35ddd 100644 --- a/src/include/catalog/pg_database.dat +++ b/src/include/catalog/pg_database.dat @@ -16,7 +16,7 @@ descr => 'default template for new databases', datname => 'template1', encoding => 'ENCODING', datlocprovider => 'LOCALE_PROVIDER', datistemplate => 't', - datallowconn => 't', dathasloginevt => 'f', datconnlimit => '-1', datfrozenxid => '0', + datallowconn => 't', dathasloginevt => 'f', dathaslogoffevt => 'f', datconnlimit => '-1', datfrozenxid => '0', datminmxid => '1', dattablespace => 'pg_default', datcollate => 'LC_COLLATE', datctype => 'LC_CTYPE', datlocale => 'DATLOCALE', daticurules => 'ICU_RULES', datacl => '_null_' }, diff --git a/src/include/catalog/pg_database.h b/src/include/catalog/pg_database.h index dbd4379ffa..98bfdb6efa 100644 --- a/src/include/catalog/pg_database.h +++ b/src/include/catalog/pg_database.h @@ -52,6 +52,9 @@ CATALOG(pg_database,1262,DatabaseRelationId) BKI_SHARED_RELATION BKI_ROWTYPE_OID /* database has login event triggers? */ bool dathasloginevt; + /* database has logoff event triggers? */ + bool dathaslogoffevt; + /* * Max connections allowed. Negative values have special meaning, see * DATCONNLIMIT_* defines below. diff --git a/src/include/commands/event_trigger.h b/src/include/commands/event_trigger.h index 90fc1af5f6..ff153347c8 100644 --- a/src/include/commands/event_trigger.h +++ b/src/include/commands/event_trigger.h @@ -57,6 +57,7 @@ extern void EventTriggerDDLCommandEnd(Node *parsetree); extern void EventTriggerSQLDrop(Node *parsetree); extern void EventTriggerTableRewrite(Node *parsetree, Oid tableOid, int reason); extern void EventTriggerOnLogin(void); +extern void EventTriggerOnLogoff(int code, Datum arg); extern bool EventTriggerBeginCompleteQuery(void); extern void EventTriggerEndCompleteQuery(void); diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index 90f9b21b25..62b190211f 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -207,6 +207,7 @@ extern PGDLLIMPORT Oid MyDatabaseId; extern PGDLLIMPORT Oid MyDatabaseTableSpace; extern PGDLLIMPORT bool MyDatabaseHasLoginEventTriggers; +extern PGDLLIMPORT bool MyDatabaseHasLogoffEventTriggers; /* * Date/Time Configuration diff --git a/src/include/tcop/cmdtaglist.h b/src/include/tcop/cmdtaglist.h index 7fdcec6dd9..0908c549cd 100644 --- a/src/include/tcop/cmdtaglist.h +++ b/src/include/tcop/cmdtaglist.h @@ -187,6 +187,7 @@ PG_CMDTAG(CMDTAG_LISTEN, "LISTEN", false, false, false) PG_CMDTAG(CMDTAG_LOAD, "LOAD", false, false, false) PG_CMDTAG(CMDTAG_LOCK_TABLE, "LOCK TABLE", false, false, false) PG_CMDTAG(CMDTAG_LOGIN, "LOGIN", true, false, false) +PG_CMDTAG(CMDTAG_LOGOFF, "LOGOFF", true, false, false) PG_CMDTAG(CMDTAG_MERGE, "MERGE", false, false, true) PG_CMDTAG(CMDTAG_MOVE, "MOVE", false, false, true) PG_CMDTAG(CMDTAG_NOTIFY, "NOTIFY", false, false, false) diff --git a/src/include/utils/evtcache.h b/src/include/utils/evtcache.h index 573e1de406..af297d3a93 100644 --- a/src/include/utils/evtcache.h +++ b/src/include/utils/evtcache.h @@ -24,6 +24,7 @@ typedef enum EVT_SQLDrop, EVT_TableRewrite, EVT_Login, + EVT_Logoff, } EventTriggerEvent; typedef struct diff --git a/src/test/regress/expected/event_trigger_logoff.out b/src/test/regress/expected/event_trigger_logoff.out new file mode 100644 index 0000000000..9e0928f964 --- /dev/null +++ b/src/test/regress/expected/event_trigger_logoff.out @@ -0,0 +1,49 @@ +-- Logoff event triggers +CREATE TABLE user_logoffs(id serial, who text); +GRANT SELECT ON user_logoffs TO public; +CREATE FUNCTION on_logoff_proc() RETURNS event_trigger AS $$ +BEGIN + INSERT INTO user_logoffs (who) VALUES (SESSION_USER); +END; +$$ LANGUAGE plpgsql; +CREATE EVENT TRIGGER on_logoff_trigger ON logoff EXECUTE FUNCTION on_logoff_proc(); +ALTER EVENT TRIGGER on_logoff_trigger ENABLE ALWAYS; +\c +-- Is it enough to wait 100ms to let the logoff event trigger execute? +SELECT pg_sleep(0.1); + pg_sleep +---------- + +(1 row) + +SELECT COUNT(*) FROM user_logoffs; + count +------- + 1 +(1 row) + +\c +SELECT pg_sleep(0.1); + pg_sleep +---------- + +(1 row) + +SELECT COUNT(*) FROM user_logoffs; + count +------- + 2 +(1 row) + +-- Check dathaslogoffevt in system catalog +SELECT dathaslogoffevt FROM pg_database WHERE datname = :'DBNAME'; + dathaslogoffevt +----------------- + t +(1 row) + +-- Cleanup +DROP TABLE user_logoffs; +DROP EVENT TRIGGER on_logoff_trigger; +DROP FUNCTION on_logoff_proc(); +\c diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule index 675c567617..e9c3d62f6d 100644 --- a/src/test/regress/parallel_schedule +++ b/src/test/regress/parallel_schedule @@ -130,6 +130,8 @@ test: oidjoins event_trigger # on-login event handling could catch connection of a concurrent test. test: event_trigger_login +test: event_trigger_logoff + # this test also uses event triggers, so likewise run it by itself test: fast_default diff --git a/src/test/regress/sql/event_trigger_logoff.sql b/src/test/regress/sql/event_trigger_logoff.sql new file mode 100644 index 0000000000..3762cea31e --- /dev/null +++ b/src/test/regress/sql/event_trigger_logoff.sql @@ -0,0 +1,26 @@ +-- Logoff event triggers +CREATE TABLE user_logoffs(id serial, who text); +GRANT SELECT ON user_logoffs TO public; +CREATE FUNCTION on_logoff_proc() RETURNS event_trigger AS $$ +BEGIN + INSERT INTO user_logoffs (who) VALUES (SESSION_USER); +END; +$$ LANGUAGE plpgsql; +CREATE EVENT TRIGGER on_logoff_trigger ON logoff EXECUTE FUNCTION on_logoff_proc(); +ALTER EVENT TRIGGER on_logoff_trigger ENABLE ALWAYS; +\c +-- Is it enough to wait 100ms to let the logoff event trigger execute? +SELECT pg_sleep(0.1); +SELECT COUNT(*) FROM user_logoffs; +\c +SELECT pg_sleep(0.1); +SELECT COUNT(*) FROM user_logoffs; + +-- Check dathaslogoffevt in system catalog +SELECT dathaslogoffevt FROM pg_database WHERE datname = :'DBNAME'; + +-- Cleanup +DROP TABLE user_logoffs; +DROP EVENT TRIGGER on_logoff_trigger; +DROP FUNCTION on_logoff_proc(); +\c base-commit: 84db9a0eb10dd1dbee6db509c0e427fa237177dc -- 2.34.1 ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2024-04-19 15:46 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-09-29 13:59 [PATCH v46 6/7] Remove the GUC stats_temp_directory Kyotaro Horiguchi <[email protected]> 2024-04-19 15:46 Support event trigger for logoff Japin Li <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox