Re: Upper limit arguments of pg_logical_slot_xxx_changes functions accept invalid values

[Brian Faherty]

The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:   tested, passed
Spec compliant:   not tested
Documentation:not tested

The error messages are nice, and I think will be a helpful feature. This patch 
does add them correctly and also tests them.

A few things I noticed, in the code, that don't really interfere with 
functionality are below:
In the `if (PG_ARGISNULL(1))` section:
I was wondering if creating an alias called something like 
InfinateInvalidXLogRecPtr might be better than adding a comment saying infinite.

In the `if (PG_ARGISNULL(2))` section:
I think '0' is being used as a magic number here and could probably have 
another #define (linked to a place to put it below). Also, it seems a little 
weird to take NULL and then just set upto_nchanges to '0' while disallowing '0' 
to be passed in as the argument. Maybe just allowing '0' as an input would be 
ok and useful to some people. I don't believe I would have noticed that if 
'upto_nchanges = UnlimitedNchanges` instead of 'upto_nchanges = 0'.

 https://doxygen.postgresql.org/xlogdefs_8h_source.html#l00028

Missing pg_control crashes postmaster

[Brian Faherty]

Hey Hackers,

If a postmaster is running and the pg_control file is removed postgres
will PANIC.

Steps to recreate:

1.) start a new cluster
2.) rm $DATADIR/pg_control
3.) psql => CHECKPOINT;

PANIC:  could not open control file "global/pg_control": No such file
or directory

After the PANIC there is no pg_control. Recovery would be difficult
without a replica or a backup. Instead of crashing we can just write a
new pg_control file since all the data is in memory at the time.

There does not really seem to be a need for this behavior as all the
information postgres needs is in memory at this point. I propose with
a patch to just recreate pg_control on updates if it does not exist.

-- 
Brian Faherty
From 80b84792d3c54a90e1e221c818a7ef025fe30b61 Mon Sep 17 00:00:00 2001
From: Brian Faherty 
Date: Mon, 23 Jul 2018 15:08:57 -0400
Subject: [PATCH] Create pg_control on update if not exists

Removing the global/pg_control file on a running postgres cluster will
no longer crash the postmaster during a checkpoint.
---
 src/backend/access/transam/xlog.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 335b4a573d..4721ea0168 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -4758,7 +4758,7 @@ UpdateControlFile(void)
 	FIN_CRC32C(ControlFile->crc);
 
 	fd = BasicOpenFile(XLOG_CONTROL_FILE,
-	   O_RDWR | PG_BINARY);
+	   O_RDWR | O_CREAT | PG_BINARY);
 	if (fd < 0)
 		ereport(PANIC,
 (errcode_for_file_access(),
-- 
2.17.1