diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index 3548c9e4987d17b0ced5afc8d572ab21e76aec8e..683ea370268a310c3d7fcb3d866d08ec9859819f 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -1331,6 +1331,25 @@ FinishPreparedTransaction(const char *gid, bool isCommit, bool raiseErrorIfNotFo gxact = LockGXact(gid, GetUserId(), raiseErrorIfNotFound); if (!raiseErrorIfNotFound && gxact == NULL) { + /* + * We can be here for commit-prepared and abort-prepared. Incase of + * commit-prepared not able to find the gxact clearly means we already + * processed the same and committed it. For abort-prepared either + * prepare was never performed on this segment hence gxact doesn't + * exists or it was performed but failed to respond back to QD. So, + * only for commit-prepared validate if it made to mirror before + * returning success to master. For abort can't detect between those 2 + * cases, hence may unnecessarily wait for mirror sync for + * abort-prepared if prepare had failed. Missing to send + * abort-prepared to mirror doesn't result in inconsistent + * result. Though yes can potentially have dangling prepared + * transaction on mirror for extremely thin window, as any transaction + * performed on primary will make sure to sync the abort prepared + * record anyways. + */ + if (isCommit) + wait_for_mirror(); + return false; } diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 6ee10c674f4a5adbb063b65180d7bdda28c597d3..daf54bc8a91b23ced806cc3d778c0336a4178f4f 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -12527,3 +12527,17 @@ last_xlog_replay_location() return recptr; } + +void +wait_for_mirror() +{ + XLogwrtResult tmpLogwrtResult; + /* use volatile pointer to prevent code rearrangement */ + volatile XLogCtlData *xlogctl = XLogCtl; + + SpinLockAcquire(&xlogctl->info_lck); + tmpLogwrtResult = xlogctl->LogwrtResult; + SpinLockRelease(&xlogctl->info_lck); + + SyncRepWaitForLSN(tmpLogwrtResult.Flush); +} diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c index a9985d5cd57c9f8e57c6609555d832653b6cadde..b3b314057c65079c3ac5aa08d5bef36ba5761ee2 100644 --- a/src/backend/replication/syncrep.c +++ b/src/backend/replication/syncrep.c @@ -294,7 +294,11 @@ SyncRepWaitForLSN(XLogRecPtr XactCommitLSN) */ if (ProcDiePending) { - ereport(WARNING, + /* + * FATAL only for QE's which use 2PC and hence can handle the + * FATAL and retry. + */ + ereport(IS_QUERY_DISPATCHER() ? WARNING:FATAL, (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."))); @@ -713,10 +717,21 @@ SyncRepQueueIsOrderedByLSN(int mode) while (proc) { /* - * Check the queue is ordered by LSN and that multiple procs don't - * have matching LSNs + * Check the queue is ordered by LSN. + * + * In upstream this check also validates that multiple procs don't + * have matching LSNs. This restriction is lifted in GPDB as for + * commit-prepared retry case since we don't know the exact lsn of + * commit-prepared record, need to wait for latest flush point + * lsn. So, its possible due to concurrency multiple backends register + * in queue with same lsn value. The check here anyways seems little + * restrictive as actual queue usage only needs it in sorted order and + * not really relies on having unique entries. It just happens to be + * that if all usage of SyncRepWaitForLSN() feed unique lsn value + * upstream and in GPDB except from FinishPreparedTransaction(), but + * not required for correct functioning of the code. */ - if (XLByteLE(proc->waitLSN, lastLSN)) + if (XLByteLT(proc->waitLSN, lastLSN)) return false; lastLSN = proc->waitLSN; diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index eba925a157a5d72f82ebc64f7f5d3b360df490f4..3956448f59c165f1bf531a810903a97e1ac6bd80 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -402,4 +402,5 @@ extern bool IsBkpBlockApplied(XLogRecord *record, uint8 block_id); extern XLogRecPtr last_xlog_replay_location(void); +extern void wait_for_mirror(void); #endif /* XLOG_H */