Hi,
> The patch changes the order to:
>
> StartTransactionCommand();
> PushActiveSnapshot(...);
> SPI_connect();
>
> ...
>
> SPI_finish();
> PopActiveSnapshot();
> CommitTransactionCommand();
>
> ... and also clarifies that the order of PushActiveSnapshot(...) and
> SPI_connect() is not important.
Additionally I noticed that the check:
```
if (!process_shared_preload_libraries_in_progress)
return;
```
... was misplaced in _PG_init(). Here is the patch v2 which fixes this too.
--
Best regards,
Aleksander Alekseev
From 4bf425356897a0c47060bb877d88049963fda814 Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <[email protected]>
Date: Fri, 12 May 2023 17:15:04 +0300
Subject: [PATCH v2] 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();
... and also clarifies that the order of PushActiveSnapshot(...) and
SPI_connect() is not important.
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 TODO FIXME
Discussion: CAJ7c6TMOhWmP92_fFss-cvNnsxdj5_TSdtmiE3AJOv6yGotoFQ@mail.gmail.com">https://postgr.es/m/CAJ7c6TMOhWmP92_fFss-cvNnsxdj5_TSdtmiE3AJOv6yGotoFQ@mail.gmail.com
---
src/test/modules/worker_spi/worker_spi.c | 16 +++++++++-------
1 file changed, 9 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..7a363bbe11 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,19 @@ 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 order of PushActiveSnapshot() and SPI_connect() is not really
+ * important.
*
* 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 +284,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 +301,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
