Revert "Add redo LSN to pgstats files"

This reverts commit b860848232aa, that was added as a prerequisite for
the support of pgstats data flush across checkpoints, linking a pgstats
file to a specific checkpoint redo LSN.

As reported, this is proving to be currently problematic when going
through a pg_upgrade, that does direct manipulations of the control file
in the new cluster.  The LSN stored in the pgstats file is not able to
cope with any changes done in the control file by pg_upgrade yet,
causing the pgstats file to be discarded when starting the new cluster
after overriding its redo LSN (one is a `pg_resetwal -l` where the new
cluster's start LSN is bumped by a hardcoded value of 8 segments, see
copy_xact_xlog_xid).

The least painful path going forward is likely going to be a refactor of
the pgstats code so as it is possible to read and write some of its data
with some routines in src/common/, so as pg_upgrade or pg_resetwal are
able to update its data.  The main point is that we are going to need a
LSN in the stats file should we make it written at checkpoint time and
not only as part of a shutdown sequence.  It is too late to dive into
these details for v18, so let's revert the change, and let's try to
figure out all the details in the next release cycle.  The pgstats file
is currently only written as part of a shutdown sequence, and its
contents are still lost on crash, same as older releases.

Bump PGSTAT_FILE_FORMAT_ID.

Reported-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/2563883.1741826489@sss.pgh.pa.us

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 799fc739e18cd8a7d122dd54adbe21968933e565..4b6c694a3f71941b190293d4ef8f346b8fdd3229 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -5728,7 +5728,7 @@ StartupXLOG(void)
    if (didCrash)
        pgstat_discard_stats();
    else
-       pgstat_restore_stats(checkPoint.redo);
+       pgstat_restore_stats();
 
    lastFullPageWrites = checkPoint.fullPageWrites;
 

diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index 3168b825e25f9c045fff701afff393b910230875..40063085073f2635662ec02329a29bb3e5b44ce3 100644 (file)
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -104,7 +104,6 @@
 #include <unistd.h>
 
 #include "access/xact.h"
-#include "access/xlog.h"
 #include "lib/dshash.h"
 #include "pgstat.h"
 #include "storage/fd.h"
@@ -181,8 +180,8 @@ typedef struct PgStat_SnapshotEntry
  * ----------
  */
 
-static void pgstat_write_statsfile(XLogRecPtr redo);
-static void pgstat_read_statsfile(XLogRecPtr redo);
+static void pgstat_write_statsfile(void);
+static void pgstat_read_statsfile(void);
 
 static void pgstat_init_snapshot_fixed(void);
 
@@ -503,9 +502,9 @@ static const PgStat_KindInfo **pgstat_kind_custom_infos = NULL;
  * Should only be called by the startup process or in single user mode.
  */
 void
-pgstat_restore_stats(XLogRecPtr redo)
+pgstat_restore_stats(void)
 {
-   pgstat_read_statsfile(redo);
+   pgstat_read_statsfile();
 }
 
 /*
@@ -581,7 +580,7 @@ pgstat_before_server_shutdown(int code, Datum arg)
    if (code == 0)
    {
        pgStatLocal.shmem->is_shutdown = true;
-       pgstat_write_statsfile(GetRedoRecPtr());
+       pgstat_write_statsfile();
    }
 }
 
@@ -1585,7 +1584,7 @@ write_chunk(FILE *fpout, void *ptr, size_t len)
  * stats so locking is not required.
  */
 static void
-pgstat_write_statsfile(XLogRecPtr redo)
+pgstat_write_statsfile(void)
 {
    FILE       *fpout;
    int32       format_id;
@@ -1602,8 +1601,7 @@ pgstat_write_statsfile(XLogRecPtr redo)
    /* we're shutting down, so it's ok to just override this */
    pgstat_fetch_consistency = PGSTAT_FETCH_CONSISTENCY_NONE;
 
-   elog(DEBUG2, "writing stats file \"%s\" with redo %X/%X", statfile,
-        LSN_FORMAT_ARGS(redo));
+   elog(DEBUG2, "writing stats file \"%s\"", statfile);
 
    /*
     * Open the statistics temp file to write out the current values.
@@ -1624,9 +1622,6 @@ pgstat_write_statsfile(XLogRecPtr redo)
    format_id = PGSTAT_FILE_FORMAT_ID;
    write_chunk_s(fpout, &format_id);
 
-   /* Write the redo LSN, used to cross check the file read */
-   write_chunk_s(fpout, &redo);
-
    /* Write various stats structs for fixed number of objects */
    for (PgStat_Kind kind = PGSTAT_KIND_MIN; kind <= PGSTAT_KIND_MAX; kind++)
    {
@@ -1773,20 +1768,18 @@ read_chunk(FILE *fpin, void *ptr, size_t len)
  * stats so locking is not required.
  */
 static void
-pgstat_read_statsfile(XLogRecPtr redo)
+pgstat_read_statsfile(void)
 {
    FILE       *fpin;
    int32       format_id;
    bool        found;
    const char *statfile = PGSTAT_STAT_PERMANENT_FILENAME;
    PgStat_ShmemControl *shmem = pgStatLocal.shmem;
-   XLogRecPtr  file_redo;
 
    /* shouldn't be called from postmaster */
    Assert(IsUnderPostmaster || !IsPostmasterEnvironment);
 
-   elog(DEBUG2, "reading stats file \"%s\" with redo %X/%X", statfile,
-        LSN_FORMAT_ARGS(redo));
+   elog(DEBUG2, "reading stats file \"%s\"", statfile);
 
    /*
     * Try to open the stats file. If it doesn't exist, the backends simply
@@ -1824,22 +1817,6 @@ pgstat_read_statsfile(XLogRecPtr redo)
        goto error;
    }
 
-   /*
-    * Read the redo LSN stored in the file.
-    */
-   if (!read_chunk_s(fpin, &file_redo))
-   {
-       elog(WARNING, "could not read redo LSN");
-       goto error;
-   }
-
-   if (file_redo != redo)
-   {
-       elog(WARNING, "found incorrect redo LSN %X/%X (expected %X/%X)",
-            LSN_FORMAT_ARGS(file_redo), LSN_FORMAT_ARGS(redo));
-       goto error;
-   }
-
    /*
     * We found an existing statistics file. Read it and put all the stats
     * data into place.

diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index def6b370ac117931bda12fed883487db0b5269d0..b01875d5b1652a45ab98bd5e2c2f71a825495c28 100644 (file)
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -11,7 +11,6 @@
 #ifndef PGSTAT_H
 #define PGSTAT_H
 
-#include "access/xlogdefs.h"
 #include "datatype/timestamp.h"
 #include "portability/instr_time.h"
 #include "postmaster/pgarch.h" /* for MAX_XFN_CHARS */
@@ -212,7 +211,7 @@ typedef struct PgStat_TableXactStatus
  * ------------------------------------------------------------
  */
 
-#define PGSTAT_FILE_FORMAT_ID  0x01A5BCB5
+#define PGSTAT_FILE_FORMAT_ID  0x01A5BCB6
 
 typedef struct PgStat_ArchiverStats
 {
@@ -514,7 +513,7 @@ extern Size StatsShmemSize(void);
 extern void StatsShmemInit(void);
 
 /* Functions called during server startup / shutdown */
-extern void pgstat_restore_stats(XLogRecPtr redo);
+extern void pgstat_restore_stats(void);
 extern void pgstat_discard_stats(void);
 extern void pgstat_before_server_shutdown(int code, Datum arg);