From daa3638a3016847a514ce71ef3eec34000945778 Mon Sep 17 00:00:00 2001 From: Aleksander Alekseev Date: Fri, 12 May 2023 17:15:04 +0300 Subject: [PATCH v3] Slight improvement of worker_spi.c example Previously the example used the following order of calls: StartTransactionCommand(); SPI_connect(); PushActiveSnapshot(...); ... SPI_finish(); PopActiveSnapshot(); CommitTransactionCommand(); This could be somewhat misleading. Typically one expects something to be freed in a reverse order comparing to initialization. This creates a false impression that PushActiveSnapshot(...) _should_ be called after SPI_connect(). The patch changes the order to: StartTransactionCommand(); PushActiveSnapshot(...); SPI_connect(); ... SPI_finish(); PopActiveSnapshot(); CommitTransactionCommand(); Additionally move the check: if (!process_shared_preload_libraries_in_progress) return; ... to the beginning of _PG_init(). The check was previously misplaced. Aleksander Alekseev, reviewed by Michael Paquier Discussion: https://postgr.es/m/CAJ7c6TMOhWmP92_fFss-cvNnsxdj5_TSdtmiE3AJOv6yGotoFQ@mail.gmail.com --- src/test/modules/worker_spi/worker_spi.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/test/modules/worker_spi/worker_spi.c b/src/test/modules/worker_spi/worker_spi.c index ad491d7722..7d52e27269 100644 --- a/src/test/modules/worker_spi/worker_spi.c +++ b/src/test/modules/worker_spi/worker_spi.c @@ -74,8 +74,8 @@ initialize_worker_spi(worktable *table) SetCurrentStatementStartTimestamp(); StartTransactionCommand(); - SPI_connect(); PushActiveSnapshot(GetTransactionSnapshot()); + SPI_connect(); pgstat_report_activity(STATE_RUNNING, "initializing worker_spi schema"); /* XXX could we use CREATE SCHEMA IF NOT EXISTS? */ @@ -222,17 +222,17 @@ worker_spi_main(Datum main_arg) * preceded by SetCurrentStatementStartTimestamp(), so that statement * start time is always up to date. * - * The SPI_connect() call lets us run queries through the SPI manager, - * and the PushActiveSnapshot() call creates an "active" snapshot + * The PushActiveSnapshot() call creates an "active" snapshot, * which is necessary for queries to have MVCC data to work on. + * The SPI_connect() call lets us run queries through the SPI manager. * * The pgstat_report_activity() call makes our activity visible * through the pgstat views. */ SetCurrentStatementStartTimestamp(); StartTransactionCommand(); - SPI_connect(); PushActiveSnapshot(GetTransactionSnapshot()); + SPI_connect(); debug_query_string = buf.data; pgstat_report_activity(STATE_RUNNING, buf.data); @@ -282,6 +282,9 @@ _PG_init(void) { BackgroundWorker worker; + if (!process_shared_preload_libraries_in_progress) + return; + /* get the configuration */ DefineCustomIntVariable("worker_spi.naptime", "Duration between each check (in seconds).", @@ -296,9 +299,6 @@ _PG_init(void) NULL, NULL); - if (!process_shared_preload_libraries_in_progress) - return; - DefineCustomIntVariable("worker_spi.total_workers", "Number of workers.", NULL, -- 2.40.1