On Tue, Sep 27, 2016 at 9:55 AM, Michael Paquier
<michael.paqu...@gmail.com> wrote:
> Seems overcomplicated to me. How about returning the control file all
> the time and let the caller pfree the result? You could then use
> crc_ok in pg_ctl.c's get_control_dbstate() to do the decision-making.
In short I would just go with the attached and call it a day.
--
Michael
diff --git a/src/backend/utils/misc/pg_controldata.c b/src/backend/utils/misc/pg_controldata.c
index 4f9de83..0c67833 100644
--- a/src/backend/utils/misc/pg_controldata.c
+++ b/src/backend/utils/misc/pg_controldata.c
@@ -34,6 +34,7 @@ pg_control_system(PG_FUNCTION_ARGS)
TupleDesc tupdesc;
HeapTuple htup;
ControlFileData *ControlFile;
+ bool crc_ok;
/*
* Construct a tuple descriptor for the result row. This must match this
@@ -51,8 +52,8 @@ pg_control_system(PG_FUNCTION_ARGS)
tupdesc = BlessTupleDesc(tupdesc);
/* read the control file */
- ControlFile = get_controlfile(DataDir, NULL);
- if (!ControlFile)
+ ControlFile = get_controlfile(DataDir, NULL, &crc_ok);
+ if (!crc_ok)
ereport(ERROR,
(errmsg("calculated CRC checksum does not match value stored in file")));
@@ -83,6 +84,7 @@ pg_control_checkpoint(PG_FUNCTION_ARGS)
ControlFileData *ControlFile;
XLogSegNo segno;
char xlogfilename[MAXFNAMELEN];
+ bool crc_ok;
/*
* Construct a tuple descriptor for the result row. This must match this
@@ -130,8 +132,8 @@ pg_control_checkpoint(PG_FUNCTION_ARGS)
tupdesc = BlessTupleDesc(tupdesc);
/* Read the control file. */
- ControlFile = get_controlfile(DataDir, NULL);
- if (!ControlFile)
+ ControlFile = get_controlfile(DataDir, NULL, &crc_ok);
+ if (!crc_ok)
ereport(ERROR,
(errmsg("calculated CRC checksum does not match value stored in file")));
@@ -216,6 +218,7 @@ pg_control_recovery(PG_FUNCTION_ARGS)
TupleDesc tupdesc;
HeapTuple htup;
ControlFileData *ControlFile;
+ bool crc_ok;
/*
* Construct a tuple descriptor for the result row. This must match this
@@ -235,8 +238,8 @@ pg_control_recovery(PG_FUNCTION_ARGS)
tupdesc = BlessTupleDesc(tupdesc);
/* read the control file */
- ControlFile = get_controlfile(DataDir, NULL);
- if (!ControlFile)
+ ControlFile = get_controlfile(DataDir, NULL, &crc_ok);
+ if (!crc_ok)
ereport(ERROR,
(errmsg("calculated CRC checksum does not match value stored in file")));
@@ -268,6 +271,7 @@ pg_control_init(PG_FUNCTION_ARGS)
TupleDesc tupdesc;
HeapTuple htup;
ControlFileData *ControlFile;
+ bool crc_ok;
/*
* Construct a tuple descriptor for the result row. This must match this
@@ -303,8 +307,8 @@ pg_control_init(PG_FUNCTION_ARGS)
tupdesc = BlessTupleDesc(tupdesc);
/* read the control file */
- ControlFile = get_controlfile(DataDir, NULL);
- if (!ControlFile)
+ ControlFile = get_controlfile(DataDir, NULL, &crc_ok);
+ if (!crc_ok)
ereport(ERROR,
(errmsg("calculated CRC checksum does not match value stored in file")));
diff --git a/src/bin/pg_controldata/pg_controldata.c b/src/bin/pg_controldata/pg_controldata.c
index e92feab..20077a6 100644
--- a/src/bin/pg_controldata/pg_controldata.c
+++ b/src/bin/pg_controldata/pg_controldata.c
@@ -86,6 +86,7 @@ int
main(int argc, char *argv[])
{
ControlFileData *ControlFile;
+ bool crc_ok;
char *DataDir = NULL;
time_t time_tmp;
char pgctime_str[128];
@@ -155,8 +156,8 @@ main(int argc, char *argv[])
}
/* get a copy of the control file */
- ControlFile = get_controlfile(DataDir, progname);
- if (!ControlFile)
+ ControlFile = get_controlfile(DataDir, progname, &crc_ok);
+ if (!crc_ok)
printf(_("WARNING: Calculated CRC checksum does not match value stored in file.\n"
"Either the file is corrupt, or it has a different layout than this program\n"
"is expecting. The results below are untrustworthy.\n\n"));
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 052b02e..ab10d2f 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -2147,28 +2147,18 @@ static DBState
get_control_dbstate(void)
{
DBState ret;
+ bool crc_ok;
+ ControlFileData *control_file_data = get_controlfile(pg_data, progname, &crc_ok);
- for (;;)
+ if (!crc_ok)
{
- ControlFileData *control_file_data = get_controlfile(pg_data, progname);
-
- if (control_file_data)
- {
- ret = control_file_data->state;
- pfree(control_file_data);
- return ret;
- }
-
- if (wait_seconds > 0)
- {
- pg_usleep(1000000); /* 1 sec */
- wait_seconds--;
- continue;
- }
-
write_stderr(_("%s: control file appears to be corrupt\n"), progname);
exit(1);
}
+
+ ret = control_file_data->state;
+ pfree(control_file_data);
+ return ret;
}
diff --git a/src/common/controldata_utils.c b/src/common/controldata_utils.c
index f218d25..822fda7 100644
--- a/src/common/controldata_utils.c
+++ b/src/common/controldata_utils.c
@@ -29,21 +29,24 @@
#include "port/pg_crc32c.h"
/*
- * get_controlfile(char *DataDir, const char *progname)
+ * get_controlfile(char *DataDir, const char *progname, bool *crc_ok_p)
*
- * Get controlfile values. The caller is responsible
- * for pfreeing the result.
+ * Get controlfile values. The result is returned as a palloc'ed copy
+ * of the control file data.
*
- * Returns NULL if the CRC did not match.
+ * crc_ok_p can be used by the caller for sanity checks to see if the
+ * CRC of the control file data is in a consistent or not.
*/
ControlFileData *
-get_controlfile(const char *DataDir, const char *progname)
+get_controlfile(const char *DataDir, const char *progname, bool *crc_ok_p)
{
ControlFileData *ControlFile;
int fd;
char ControlFilePath[MAXPGPATH];
pg_crc32c crc;
+ Assert(crc_ok_p != NULL);
+
ControlFile = palloc(sizeof(ControlFileData));
snprintf(ControlFilePath, MAXPGPATH, "%s/global/pg_control", DataDir);
@@ -83,11 +86,7 @@ get_controlfile(const char *DataDir, const char *progname)
offsetof(ControlFileData, crc));
FIN_CRC32C(crc);
- if (!EQ_CRC32C(crc, ControlFile->crc))
- {
- pfree(ControlFile);
- return NULL;
- }
+ *crc_ok_p = EQ_CRC32C(crc, ControlFile->crc);
/* Make sure the control file is valid byte order. */
if (ControlFile->pg_control_version % 65536 == 0 &&
diff --git a/src/include/common/controldata_utils.h b/src/include/common/controldata_utils.h
index f834624..a822cba 100644
--- a/src/include/common/controldata_utils.h
+++ b/src/include/common/controldata_utils.h
@@ -12,6 +12,6 @@
#include "catalog/pg_control.h"
-extern ControlFileData *get_controlfile(const char *DataDir, const char *progname);
+extern ControlFileData *get_controlfile(const char *DataDir, const char *progname, bool *crc_ok_p);
#endif /* COMMON_CONTROLDATA_UTILS_H */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
