Misc cleanup in datachecksums_state.[ch]

Move DataChecksumsWorkerResult struct to the .c file.  It's not used
anywhere else since commit 07009121c2 removed the injection point test
code that the comment referred to.

Mark StartDataChecksumsWorkerLauncher() as static, since it's not
called from outside the .c file.  The DataChecksumsWorkerOperation
struct can then be moved into the .c file too.

Clarify the comment on StartDataChecksumsWorkerLauncher().  It said
"Main entry point for datachecksumsworker launcher process", but I
found that misleading.  That description would be a better fit for
DataChecksumsWorkerLauncherMain(), which is the process's "main"
function, rather than StartDataChecksumsWorkerLauncher().

Fix comment on WaitForAllTransactionsToFinish() on postmaster death.
The comment claimed that it sets "the abort flag" on postmaster death,
but it actually just errors outs.  Improve the comment to explain why
it doesn't just use WL_EXIT_ON_PM_DEATH.

Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://www.postgresql.org/message-id/b283fbb9-298e-4953-9120-eefaf24fae20@iki.fi

diff --git a/src/backend/postmaster/datachecksum_state.c b/src/backend/postmaster/datachecksum_state.c
index 04f1a268845882ed8b0ebdb5ed7b5d5dee217a57..9103546c3825c88a587f651af3bdf6b7c73ca07d 100644 (file)
--- a/src/backend/postmaster/datachecksum_state.c
+++ b/src/backend/postmaster/datachecksum_state.c
@@ -276,6 +276,22 @@ static const ChecksumBarrierCondition checksum_barriers[9] =
        {PG_DATA_CHECKSUM_OFF, PG_DATA_CHECKSUM_INPROGRESS_OFF},
 };
 
+/* Possible operations the DataChecksumsWorker can perform */
+typedef enum DataChecksumsWorkerOperation
+{
+       ENABLE_DATACHECKSUMS,
+       DISABLE_DATACHECKSUMS,
+} DataChecksumsWorkerOperation;
+
+/* Possible states for a database entry which has been processed */
+typedef enum
+{
+       DATACHECKSUMSWORKER_SUCCESSFUL = 0,
+       DATACHECKSUMSWORKER_ABORTED,
+       DATACHECKSUMSWORKER_FAILED,
+       DATACHECKSUMSWORKER_DROPDB,
+} DataChecksumsWorkerResult;
+
 /*
  * Signaling between backends calling pg_enable/disable_data_checksums, the
  * checksums launcher process, and the checksums worker process.
@@ -355,6 +371,9 @@ static volatile sig_atomic_t launcher_running = false;
 static DataChecksumsWorkerOperation operation;
 
 /* Prototypes */
+static void StartDataChecksumsWorkerLauncher(DataChecksumsWorkerOperation op,
+                                                                                        int cost_delay,
+                                                                                        int cost_limit);
 static void DataChecksumsShmemRequest(void *arg);
 static bool DatabaseExists(Oid dboid);
 static List *BuildDatabaseList(void);
@@ -555,12 +574,12 @@ enable_data_checksums(PG_FUNCTION_ARGS)
 
 /*
  * StartDataChecksumsWorkerLauncher
- *             Main entry point for datachecksumsworker launcher process
+ *             Start the datachecksumsworker launcher process, if not running yet
  *
- * The main entrypoint for starting data checksums processing for enabling as
- * well as disabling.
+ * This is called to start data checksums processing for enabling as well as
+ * disabling.
  */
-void
+static void
 StartDataChecksumsWorkerLauncher(DataChecksumsWorkerOperation op,
                                                                 int cost_delay,
                                                                 int cost_limit)
@@ -979,9 +998,7 @@ launcher_cancel_handler(SIGNAL_ARGS)
  *             Blocks awaiting all current transactions to finish
  *
  * Returns when all transactions which are active at the call of the function
- * have ended, or if the postmaster dies while waiting. If the postmaster dies
- * the abort flag will be set to indicate that the caller of this shouldn't
- * proceed.
+ * have ended.
  *
  * NB: this will return early, if aborted by SIGINT or if the target state
  * is changed while we're running.
@@ -1015,8 +1032,8 @@ WaitForAllTransactionsToFinish(void)
                                           WAIT_EVENT_CHECKSUM_ENABLE_STARTCONDITION);
 
                /*
-                * If the postmaster died we won't be able to enable checksums
-                * cluster-wide so abort and hope to continue when restarted.
+                * If the postmaster died, bail out.  But first print a log message to
+                * note that the checksumming didn't complete.
                 */
                if (rc & WL_POSTMASTER_DEATH)
                        ereport(FATAL,

diff --git a/src/include/postmaster/datachecksum_state.h b/src/include/postmaster/datachecksum_state.h
index 2a1ae10d55d2029233e8e947327ef07696f8ad36..f34db0c09e07cceff7d9ca17bce04e5cda6df49b 100644 (file)
--- a/src/include/postmaster/datachecksum_state.h
+++ b/src/include/postmaster/datachecksum_state.h
@@ -17,36 +17,10 @@
 
 #include "storage/procsignal.h"
 
-/* Possible operations the DataChecksumsWorker can perform */
-typedef enum DataChecksumsWorkerOperation
-{
-       ENABLE_DATACHECKSUMS,
-       DISABLE_DATACHECKSUMS,
-} DataChecksumsWorkerOperation;
-
-/*
- * Possible states for a database entry which has been processed. Exported
- * here since we want to be able to reference this from injection point tests.
- */
-typedef enum
-{
-       DATACHECKSUMSWORKER_SUCCESSFUL = 0,
-       DATACHECKSUMSWORKER_ABORTED,
-       DATACHECKSUMSWORKER_FAILED,
-       DATACHECKSUMSWORKER_DROPDB,
-} DataChecksumsWorkerResult;
-
 /* Prototypes for data checksum state manipulation */
 bool           AbsorbDataChecksumsBarrier(ProcSignalBarrierType barrier);
 void           EmitAndWaitDataChecksumsBarrier(uint32 state);
 
-/* Prototypes for data checksum background worker */
-
-/* Start the background processes for enabling or disabling checksums */
-void           StartDataChecksumsWorkerLauncher(DataChecksumsWorkerOperation op,
-                                                                                        int cost_delay,
-                                                                                        int cost_limit);
-
 /* Background worker entrypoints */
 void           DataChecksumsWorkerLauncherMain(Datum arg);
 void           DataChecksumsWorkerMain(Datum arg);