Fix stuck query after cancel or termination when segment is not respo…

…nding (#948)

Problem:
The following scenario caused a stuck state of a coordinator backend process:
1. At some moment one of the segments stopped processing incoming requests (the 
reason itself is not important, as sometimes bad things may happen with any
segment, and the system should be able to recover).
2. A query, which dispatches requests to segments, was executed (for example,
select from 'gp_toolkit.gp_resgroup_status_per_segment'). As one of the segments 
was not responding, the coordinator hanged in 'checkDispatchResult' (it is 
expected, and can be handled by the FTS).
3. The stuck query was canceled or terminated (by 'pg_cancel_backend' or 
'pg_terminate_backend'). It didn't help to return from the stuck state, but
after this step the query became completely unrecoverable. Even after FTS had
detected the malfunction segment and had promoted the mirror, the query was
still hanging forever. 

Expected behavior is that after FTS mirror promotion, all stuck queries are 
unblocked and canceled successfully. 

Note: if FTS mirror promotion happened before step 3, the FTS canceled the 
query successfully.

Root cause:
During cancel or termination, the coordinator tried to do abort of the current
transaction, and it hanged in the function 'internal_cancel' (called from 
PQcancel) on 'poll' system call. It has a timeout of 660 seconds, and, moreover,
after the timeout expired, it looped forever trying to do 'poll' again. So, if 
the socket was opened on the segment side, but nobody replied (as the segment 
process became not available for some reason), 'internal_cancel' had no way to
return.

Fix:
Once FTS promotes a mirror, it sends the  signal to the coordinator
postmaster (with PMSIGNAL_FTS_PROMOTED_MIRROR reason). On receiving of the
signal, the coordinator postmaster sends the SIGUSR1 signal (with 
PROCSIG_FTS_PROMOTED_MIRROR reason) to all of its usual backends. Once the 
backend receives the signal, if it is in the state of cancelling or terminating
of the query, it sets a flag in libpq. 'internal_cancel' checks this flag before
calling the 'poll' system call. If it is set, it will return with an error.
Thus:
a. if the FTS promotion happens before cancel/terminate, the query will be 
canceled by the old logic;
b. if the FTS promotion happens after cancel/terminate, but before the 
'internal_cancel' calls the 'poll', 'internal_cancel' will return an error
without calling the 'poll';
c. if the FTS promotion happens when the 'poll' is already called, the 'poll' 
will return EINTR (as the SIGUSR1 was received), and a new 'poll' will not be
called, as the flag is set.

src/backend/cdb/cdbutil.c

@@ -662,6 +662,9 @@ cdbcomponent_cleanupIdleQEs(bool includeWriter)
 		}
 	}
 
+	/* reset flag in libpq, that is used to avoid stuck in a loop in PQcancel*/
+	PQbypassConnCloseAtCancel(false);
+
 	return;
 }
 

src/backend/fts/ftsprobe.c

@@ -29,6 +29,7 @@
 #include "postmaster/fts.h"
 #include "postmaster/ftsprobe.h"
 #include "postmaster/postmaster.h"
+#include "storage/pmsignal.h"
 #include "utils/snapmgr.h"
 
 
@@ -1175,6 +1176,7 @@ processResponse(fts_context *context)
 					   "triggered successfully",
 					   primary->config->segindex, primary->config->dbid);
 				ftsInfo->state = FTS_RESPONSE_PROCESSED;
+				SendPostmasterSignal(PMSIGNAL_FTS_PROMOTED_MIRROR);
 				break;
 			case FTS_SYNCREP_OFF_SUCCESS:
 				elogif(gp_log_fts >= GPVARS_VERBOSITY_VERBOSE, LOG,

src/backend/postmaster/postmaster.c

@@ -5690,6 +5690,29 @@ sigusr1_handler(SIGNAL_ARGS)
 		signal_child(DtxRecoveryPID(), SIGINT);
 	}
 
+	if (CheckPostmasterSignal(PMSIGNAL_FTS_PROMOTED_MIRROR))
+	{
+		/*
+		 * Notify all child coordinator backends that FTS has promoted a mirror.
+		 */
+		if (pmState == PM_RUN &&
+			GpIdentity.segindex == MASTER_CONTENT_ID)
+		{
+			dlist_iter	iter;
+			dlist_foreach(iter, &BackendList)
+			{
+				Backend *bp = dlist_container(Backend, elem, iter.cur);
+
+				if (bp->dead_end || !(BACKEND_TYPE_NORMAL & bp->bkend_type))
+					continue;
+
+				SendProcSignal(bp->pid,
+							   PROCSIG_FTS_PROMOTED_MIRROR,
+							   InvalidBackendId);
+			}
+		}
+	}
+
 	/*
 	 * Try to advance postmaster's state machine, if a child requests it.
 	 *

src/backend/storage/ipc/procsignal.c

@@ -19,6 +19,7 @@
 
 #include "cdb/cdbvars.h"
 #include "commands/async.h"
+#include "libpq-fe.h"
 #include "miscadmin.h"
 #include "replication/walsender.h"
 #include "storage/latch.h"
@@ -288,6 +289,23 @@ QueryFinishHandler(void)
 	}
 }
 
+/*
+ * Coordinator postmaster signals that FTS has detected a failed segment and 
+ * promoted the mirror.  
+ */
+static void
+FtsPromotedMirrorHandler(void)
+{
+	/*
+	 * In case the promotion is done during cancel or termination of a query,
+	 * there is a very high chance that libpq will be stuck forever trying to
+	 * wait cancel confirmation from the segment, which is not responding. Code
+	 * below handles this case.
+	 */
+	if (QueryCancelCleanup || TermSignalReceived)
+		PQbypassConnCloseAtCancel(true);
+}
+
 /*
  * procsignal_sigusr1_handler - handle SIGUSR1 signal.
  */
@@ -329,6 +347,9 @@ procsignal_sigusr1_handler(SIGNAL_ARGS)
 	if (CheckProcSignal(PROCSIG_RESOURCE_GROUP_MOVE_QUERY))
 		HandleMoveResourceGroup();
 
+	if (CheckProcSignal(PROCSIG_FTS_PROMOTED_MIRROR))
+		FtsPromotedMirrorHandler();
+
 	if (set_latch_on_sigusr1 && MyProc != NULL)
 		SetLatch(&MyProc->procLatch);
 

src/include/storage/pmsignal.h

@@ -35,6 +35,8 @@ typedef enum
 	PMSIGNAL_WAKEN_FTS,         /* wake up FTS to probe segments */
 	PMSIGNAL_WAKEN_DTX_RECOVERY,         /* wake up dtx recovery to abort dtx xacts */
 
+	PMSIGNAL_FTS_PROMOTED_MIRROR, /* FTS has detected failed primary and promoted mirror*/
+
 	NUM_PMSIGNALS				/* Must be last value of enum! */
 } PMSignalReason;
 

src/include/storage/procsignal.h

@@ -43,6 +43,9 @@ typedef enum
 	PROCSIG_RECOVERY_CONFLICT_BUFFERPIN,
 	PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK,
 	PROCSIG_RESOURCE_GROUP_MOVE_QUERY,	/* move query to a new resource group */
+
+	PROCSIG_FTS_PROMOTED_MIRROR, /* FTS has detected failed primary and promoted mirror*/
+
 	NUM_PROCSIGNALS				/* Must be last! */
 } ProcSignalReason;
 

src/interfaces/libpq/fe-connect.c

@@ -391,6 +391,8 @@ static const PQEnvironmentOption EnvironmentOptions[] =
 static const char uri_designator[] = "postgresql://";
 static const char short_uri_designator[] = "postgres://";
 
+static bool bypass_conn_close_at_cancel = false;
+
 static bool connectOptions1(PGconn *conn, const char *conninfo);
 static bool connectOptions2(PGconn *conn);
 static int	connectDBStart(PGconn *conn);
@@ -3429,6 +3431,15 @@ PQfreeCancel(PGcancel *cancel)
 		free(cancel);
 }
 
+/*
+ * PQbypassConnCloseAtCancel:
+ * set a flag, that allows to fix an issue with internal hanging in
+ * 'internal_cancel'. For details refer to comments in 'internal_cancel'.
+ */
+void PQbypassConnCloseAtCancel(pqbool bypass)
+{
+	bypass_conn_close_at_cancel = bypass;
+}
 
 /*
  * PQcancel and PQrequestCancel: attempt to request cancellation of the
@@ -3523,6 +3534,26 @@ internal_cancel(SockAddr *raddr, int be_pid, int be_key,
 	 */
 #ifndef WIN32
 retry5:
+
+	/*
+	 * GPDB: in case this function is called by the coordinator process to
+	 * communicate with the segments (when performing cancel/terminate of a 
+	 * backend) and one of the segments has hanged keeping the listening socket
+	 * open, we will be stuck in this function for infinite amount of time
+	 * (hanging on the 'poll' in a loop). The check below helps to avoid this
+	 * situation:
+	 *  1. The flag 'bypass_conn_close_at_cancel' is set from a signal handler.
+	 *  2. The signal is originally initiated by the FTS, which detects that a
+	 *  segment went down. 
+	 *  3. If the signal comes before the 'poll' is called, it will be just
+	 *  skipped and this function will return an error.
+	 *  4. If the signal comes after the 'poll' is called, the 'poll' will be
+	 *  interrupted with EINTR, and the next iteration will be skipped and this
+	 *  function will return an error.
+	 */
+	if (bypass_conn_close_at_cancel)
+		goto cancel_errReturn;
+
 	pollFds[0].fd = tmpsock;
 	pollFds[0].events = POLLIN;
 	pollFds[0].revents = 0;

src/interfaces/libpq/libpq-fe.h

@@ -325,6 +325,8 @@ extern void PQfreeCancel(PGcancel *cancel);
 /* issue a cancel request */
 extern int	PQcancel(PGcancel *cancel, char *errbuf, int errbufsize);
 
+extern void PQbypassConnCloseAtCancel(pqbool bypass); /* GPDB only */
+
 /* issue a finsh request */
 extern int	PQrequestFinish(PGcancel *cancel, char *errbuf, int errbufsize);