On Tue, 2021-04-20 at 14:19 -0700, SATYANARAYANA NARLAPURAM wrote:
> One idea here is to make the backend ignore query
> cancellation/backend termination while waiting for the synchronous
> commit ACK. This way client never reads the data that was never
> flushed remotely. The problem with this approach is that your
> backends get stuck until your commit log record is flushed on the
> remote side. Also, the client can see the data not flushed remotely
> if the server crashes and comes back online. You can prevent the
> latter case by making a SyncRepWaitForLSN before opening up the
> connections to the non-superusers. I have a working prototype of this
> logic, if there is enough interest I can post the patch.
I didn't see a patch here yet, so I wrote a simple one for
consideration (attached).
The problem exists for both cancellation and termination requests. The
patch adds a GUC that makes SyncRepWaitForLSN keep waiting. It does not
ignore the requests; for instance, a termination request will still be
honored when it's done waiting for sync rep.
The idea of this GUC is not to wait forever (obviously), but to allow
the administrator (or an automated network agent) to be in control of
the logic:
If the primary is non-responsive, the administrator can decide to fail
over, knowing that all visible transactions on the primary are durable
on the standby (because any transaction that didn't make it to the
standby also didn't release locks yet). If the standby is non-
responsive, the administrator can intervene with something like:
ALTER SYSTEM SET synchronous_standby_names = '';
SELECT pg_reload_conf();
which will disable sync rep, allowing the primary to complete the query
and continue on without the standby; but in that case the admin must be
sure not to fail over until there's a new standby fully caught-up.
The patch may be somewhat controversial, so I'll wait for feedback
before documenting it properly.
Regards,
Jeff Davis
diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c
index bdbc9ef844b..4ca96e59bb6 100644
--- a/src/backend/replication/syncrep.c
+++ b/src/backend/replication/syncrep.c
@@ -88,6 +88,7 @@
/* User-settable parameters for sync rep */
char *SyncRepStandbyNames;
+bool synchronous_replication_interrupt = true;;
#define SyncStandbysDefined() \
(SyncRepStandbyNames != NULL && SyncRepStandbyNames[0] != '\0')
@@ -150,6 +151,8 @@ SyncRepWaitForLSN(XLogRecPtr lsn, bool commit)
char *new_status = NULL;
const char *old_status;
int mode;
+ bool termination_warning_emitted = false;
+ bool cancel_warning_emitted = false;
/*
* This should be called while holding interrupts during a transaction
@@ -263,13 +266,24 @@ SyncRepWaitForLSN(XLogRecPtr lsn, bool commit)
*/
if (ProcDiePending)
{
- ereport(WARNING,
- (errcode(ERRCODE_ADMIN_SHUTDOWN),
- errmsg("canceling the wait for synchronous replication and terminating connection due to administrator command"),
- errdetail("The transaction has already committed locally, but might not have been replicated to the standby.")));
- whereToSendOutput = DestNone;
- SyncRepCancelWait();
- break;
+ if (synchronous_replication_interrupt) {
+ ereport(WARNING,
+ (errcode(ERRCODE_ADMIN_SHUTDOWN),
+ errmsg("canceling the wait for synchronous replication and terminating connection due to administrator command"),
+ errdetail("The transaction has already committed locally, but might not have been replicated to the standby.")));
+ whereToSendOutput = DestNone;
+ SyncRepCancelWait();
+ break;
+ }
+ else if (!termination_warning_emitted)
+ {
+ ereport(WARNING,
+ (errcode(ERRCODE_ADMIN_SHUTDOWN),
+ errmsg("received request to terminate connection while waiting for synchronous replication"),
+ errdetail("The transaction has already committed locally, but might not have been replicated to the standby."),
+ errhint("Set synchronous_standby_names='' and reload the configuration to allow termination to proceed.")));
+ termination_warning_emitted = true;
+ }
}
/*
@@ -280,12 +294,24 @@ SyncRepWaitForLSN(XLogRecPtr lsn, bool commit)
*/
if (QueryCancelPending)
{
- QueryCancelPending = false;
- ereport(WARNING,
- (errmsg("canceling wait for synchronous replication due to user request"),
- errdetail("The transaction has already committed locally, but might not have been replicated to the standby.")));
- SyncRepCancelWait();
- break;
+ if (synchronous_replication_interrupt)
+ {
+ QueryCancelPending = false;
+ ereport(WARNING,
+ (errmsg("canceling wait for synchronous replication due to user request"),
+ errdetail("The transaction has already committed locally, but might not have been replicated to the standby.")));
+ SyncRepCancelWait();
+ break;
+ }
+ else if (!cancel_warning_emitted)
+ {
+ ereport(WARNING,
+ (errcode(ERRCODE_ADMIN_SHUTDOWN),
+ errmsg("received cancellation request while waiting for synchronous replication"),
+ errdetail("The transaction has already committed locally, but might not have been replicated to the standby."),
+ errhint("Set synchronous_standby_names='' and reload the configuration to allow the client to proceed.")));
+ cancel_warning_emitted = true;
+ }
}
/*
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 68b62d523dc..577d3cc29f5 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1294,6 +1294,16 @@ static struct config_bool ConfigureNamesBool[] =
NULL, NULL, NULL
},
+ {
+ {"synchronous_replication_interrupt", PGC_POSTMASTER, WAL_SETTINGS,
+ gettext_noop("Enable interrupt while waiting for synchronous replication."),
+ NULL
+ },
+ &synchronous_replication_interrupt,
+ true,
+ NULL, NULL, NULL
+ },
+
{
{"wal_log_hints", PGC_POSTMASTER, WAL_SETTINGS,
gettext_noop("Writes full pages to WAL when first modified after a checkpoint, even for a non-critical modification."),
diff --git a/src/include/replication/syncrep.h b/src/include/replication/syncrep.h
index 4266afde8be..054bc60b30c 100644
--- a/src/include/replication/syncrep.h
+++ b/src/include/replication/syncrep.h
@@ -80,6 +80,7 @@ extern char *syncrep_parse_error_msg;
/* user-settable parameters for synchronous replication */
extern char *SyncRepStandbyNames;
+extern bool synchronous_replication_interrupt;
/* called by user backend */
extern void SyncRepWaitForLSN(XLogRecPtr lsn, bool commit);
