From c7c5b6d2635b819b912219338016cca5eda82e11 Mon Sep 17 00:00:00 2001
From: Jimmy Yih <jyih@pivotal.io>
Date: Wed, 22 Nov 2017 13:20:33 -0800
Subject: [PATCH] Only allow fts probe process to touch fts probe pause GUC

Some master processes were calling gpvars_assign_gp_fts_probe_pause
when they should not. This was found when the new stats collecter
process errored out on this function call. We also move it to fts.c
where it will live alongside the other fts code.

Also, check for interrupts in fts sleep loop so that fts probe process
can process SIGHUP properly.
---
 src/backend/cdb/cdbvars.c       | 39 ---------------------------------
 src/backend/fts/fts.c           | 33 +++++++++++++++++++++++++++-
 src/backend/utils/misc/guc_gp.c |  1 +
 src/include/cdb/cdbvars.h       |  3 ---
 src/include/postmaster/fts.h    |  4 ++++
 5 files changed, 37 insertions(+), 43 deletions(-)

diff --git a/src/backend/cdb/cdbvars.c b/src/backend/cdb/cdbvars.c
index 381e57e3af..aefb39ccd7 100644
--- a/src/backend/cdb/cdbvars.c
+++ b/src/backend/cdb/cdbvars.c
@@ -1179,45 +1179,6 @@ gpvars_show_gp_gpperfmon_log_alert_level(void)
 	return gpperfmon_log_alert_level_to_string(gpperfmon_log_alert_level);
 }
 
-/*
- * Request the fault-prober to suspend probes -- no fault actions will
- * be taken based on in-flight probes until the prober is unpaused.
- */
-bool
-gpvars_assign_gp_fts_probe_pause(bool newval, bool doit, GucSource source)
-{
-	if (doit)
-	{
-		/*
-		 * We only want to do fancy stuff on the master (where we have a
-		 * prober).
-		 */
-		if (ftsProbeInfo && Gp_segment == -1)
-		{
-			/*
-			 * fts_pauseProbes is externally set/cleared; fts_cancelProbes is
-			 * externally set and cleared by FTS
-			 */
-			ftsLock();
-			ftsProbeInfo->fts_pauseProbes = newval;
-			ftsProbeInfo->fts_discardResults = ftsProbeInfo->fts_discardResults || newval;
-			ftsUnlock();
-
-			/*
-			 * If we're unpausing, we want to force the prober to re-read
-			 * everything. (we want FtsNotifyProber()).
-			 */
-			if (!newval)
-			{
-				FtsNotifyProber();
-			}
-		}
-		gp_fts_probe_pause = newval;
-	}
-
-	return true;
-}
-
 /*
  * gpvars_assign_gp_resource_manager_policy
  * gpvars_show_gp_resource_manager_policy
diff --git a/src/backend/fts/fts.c b/src/backend/fts/fts.c
index 1dcb8fb00a..e81e20dec7 100644
--- a/src/backend/fts/fts.c
+++ b/src/backend/fts/fts.c
@@ -795,6 +795,8 @@ void FtsLoop()
 			if (elapsed < gp_fts_probe_interval && !shutdown_requested)
 			{
 				pg_usleep((gp_fts_probe_interval - elapsed) * USECS_PER_SEC);
+
+				CHECK_FOR_INTERRUPTS();
 			}
 		}
 	} /* end server loop */
@@ -898,4 +900,33 @@ void FtsRequestPostmasterShutdown(CdbComponentDatabaseInfo *primary, CdbComponen
 			    );
 }
 
-/* EOF */
+/*
+ * Request the fault-prober to suspend probes -- no fault actions will
+ * be taken based on in-flight probes until the prober is unpaused.
+ */
+bool
+gpvars_assign_gp_fts_probe_pause(bool newval, bool doit, GucSource source)
+{
+	if (doit)
+	{
+		/*
+		 * We only want to do fancy stuff on the master (where we have a
+		 * prober).
+		 */
+		if (am_ftsprobe && ftsProbeInfo && Gp_segment == -1)
+		{
+			/*
+			 * fts_pauseProbes is externally set/cleared; fts_cancelProbes is
+			 * externally set and cleared by FTS
+			 */
+			ftsLock();
+
+			ftsProbeInfo->fts_pauseProbes = newval;
+			ftsProbeInfo->fts_discardResults = ftsProbeInfo->fts_discardResults || newval;
+			ftsUnlock();
+		}
+		gp_fts_probe_pause = newval;
+	}
+
+	return true;
+}
diff --git a/src/backend/utils/misc/guc_gp.c b/src/backend/utils/misc/guc_gp.c
index dc4f5a9a39..d729622abe 100644
--- a/src/backend/utils/misc/guc_gp.c
+++ b/src/backend/utils/misc/guc_gp.c
@@ -36,6 +36,7 @@
 #include "pgstat.h"
 #include "parser/scansup.h"
 #include "postmaster/syslogger.h"
+#include "postmaster/fts.h"
 #include "replication/walsender.h"
 #include "storage/bfz.h"
 #include "storage/proc.h"
diff --git a/src/include/cdb/cdbvars.h b/src/include/cdb/cdbvars.h
index 025b0cda97..e7f864031a 100644
--- a/src/include/cdb/cdbvars.h
+++ b/src/include/cdb/cdbvars.h
@@ -319,9 +319,6 @@ extern bool gp_fts_probe_pause;
 extern int gp_fts_transition_retries;
 extern int gp_fts_transition_timeout;
 
-extern bool gpvars_assign_gp_fts_probe_pause(bool newval, bool doit, GucSource source);
-
-
 /*
  * Parameter gp_connections_per_thread
  *
diff --git a/src/include/postmaster/fts.h b/src/include/postmaster/fts.h
index ffa7976d9d..19aaa1130d 100644
--- a/src/include/postmaster/fts.h
+++ b/src/include/postmaster/fts.h
@@ -16,6 +16,7 @@
 #ifndef FTS_H
 #define FTS_H
 
+#include "utils/guc.h"
 #include "cdb/cdbutil.h"
 
 #ifdef USE_SEGWALREP
@@ -185,6 +186,9 @@ extern void FtsRequestPostmasterShutdown(CdbComponentDatabaseInfo *primary, CdbC
 extern bool FtsMasterShutdownRequested(void);
 extern void FtsRequestMasterShutdown(void);
 
+/* Interface for setting FTS GUCs */
+extern bool gpvars_assign_gp_fts_probe_pause(bool newval, bool doit, GucSource source);
+
 /*
  * If master has requested FTS to shutdown.
  */
-- 
GitLab