Merge pull request #1836 from citusdata/fix_vacuum_analyze_propagation

Ensure VACUUM/ANALYZE stays local when unsupported or DDL prop disabled

cr: @pykello

src/backend/distributed/executor/multi_utility.c

@@ -1534,39 +1534,39 @@ static bool
 IsSupportedDistributedVacuumStmt(Oid relationId, VacuumStmt *vacuumStmt)
 {
 	const char *stmtName = (vacuumStmt->options & VACOPT_VACUUM) ? "VACUUM" : "ANALYZE";
+	bool distributeStmt = false;
 
 	if (vacuumStmt->relation == NULL)
 	{
-		/* WARN and exit early for unqualified VACUUM commands */
+		/* WARN for unqualified VACUUM commands */
 		ereport(WARNING, (errmsg("not propagating %s command to worker nodes", stmtName),
 						  errhint("Provide a specific table in order to %s "
 								  "distributed tables.", stmtName)));
-
-		return false;
 	}
-
+	else if (!OidIsValid(relationId) || !IsDistributedTable(relationId))
-	if (!OidIsValid(relationId) || !IsDistributedTable(relationId))
 	{
-		return false;
+		/* Nothing to do here; relation no longer exists or is not distributed */
 	}
-
+	else if (!EnableDDLPropagation)
-	if (!EnableDDLPropagation)
 	{
-		/* WARN and exit early if DDL propagation is not enabled */
+		/* WARN if DDL propagation is not enabled */
 		ereport(WARNING, (errmsg("not propagating %s command to worker nodes", stmtName),
 						  errhint("Set citus.enable_ddl_propagation to true in order to "
 								  "send targeted %s commands to worker nodes.",
 								  stmtName)));
 	}
-
+	else if (vacuumStmt->options & VACOPT_VERBOSE)
-	if (vacuumStmt->options & VACOPT_VERBOSE)
 	{
 		ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 						errmsg("the VERBOSE option is currently unsupported in "
 							   "distributed %s commands", stmtName)));
 	}
+	else
+	{
+		distributeStmt = true;
+	}
 
-	return true;
+	return distributeStmt;
 }
 
 

src/test/regress/expected/multi_utilities.out

@@ -210,6 +210,45 @@ begin
     extract(epoch from clock_timestamp() - start_time);
 end
 $$ language plpgsql;
+\c - - - :worker_2_port
+CREATE MATERIALIZED VIEW prevcounts AS
+SELECT analyze_count, vacuum_count FROM pg_stat_user_tables
+WHERE relname='dustbunnies_990001';
+-- create function that sleeps until those counters increment
+create function wait_for_stats() returns void as $$
+declare
+  start_time timestamptz := clock_timestamp();
+  analyze_updated bool;
+  vacuum_updated bool;
+begin
+  -- we don't want to wait forever; loop will exit after 10 seconds
+  for i in 1 .. 100 loop
+
+    -- check to see if analyze has been updated
+    SELECT (st.analyze_count >= pc.analyze_count + 1) INTO analyze_updated
+      FROM pg_stat_user_tables AS st, pg_class AS cl, prevcounts AS pc
+     WHERE st.relname='dustbunnies_990001' AND cl.relname='dustbunnies_990001';
+
+     -- check to see if vacuum has been updated
+    SELECT (st.vacuum_count >= pc.vacuum_count + 1) INTO vacuum_updated
+      FROM pg_stat_user_tables AS st, pg_class AS cl, prevcounts AS pc
+     WHERE st.relname='dustbunnies_990001' AND cl.relname='dustbunnies_990001';
+
+    exit when analyze_updated or vacuum_updated;
+
+    -- wait a little
+    perform pg_sleep(0.1);
+
+    -- reset stats snapshot so we can test again
+    perform pg_stat_clear_snapshot();
+
+  end loop;
+
+  -- report time waited in postmaster log (where it won't change test output)
+  raise log 'wait_for_stats delayed % seconds',
+    extract(epoch from clock_timestamp() - start_time);
+end
+$$ language plpgsql;
 -- run VACUUM and ANALYZE against the table on the master
 \c - - - :master_port
 VACUUM dustbunnies;
@@ -317,12 +356,59 @@ WHERE tablename = 'dustbunnies_990002' ORDER BY attname;
 VACUUM;
 WARNING:  not propagating VACUUM command to worker nodes
 HINT:  Provide a specific table in order to VACUUM distributed tables.
+-- check the current number of vacuum and analyze run on dustbunnies
+SELECT run_command_on_workers($$SELECT wait_for_stats()$$);
+ run_command_on_workers 
+------------------------
+ (localhost,57637,t,"")
+ (localhost,57638,t,"")
+(2 rows)
+
+SELECT run_command_on_workers($$SELECT pg_stat_get_vacuum_count(tablename::regclass) from pg_tables where tablename LIKE 'dustbunnies_%' limit 1$$);
+ run_command_on_workers 
+------------------------
+ (localhost,57637,t,3)
+ (localhost,57638,t,3)
+(2 rows)
+
+SELECT run_command_on_workers($$SELECT pg_stat_get_analyze_count(tablename::regclass) from pg_tables where tablename LIKE 'dustbunnies_%' limit 1$$);
+ run_command_on_workers 
+------------------------
+ (localhost,57637,t,3)
+ (localhost,57638,t,3)
+(2 rows)
+
 -- and warning when using targeted VACUUM without DDL propagation
 SET citus.enable_ddl_propagation to false;
 VACUUM dustbunnies;
 WARNING:  not propagating VACUUM command to worker nodes
 HINT:  Set citus.enable_ddl_propagation to true in order to send targeted VACUUM commands to worker nodes.
+ANALYZE dustbunnies;
+WARNING:  not propagating ANALYZE command to worker nodes
+HINT:  Set citus.enable_ddl_propagation to true in order to send targeted ANALYZE commands to worker nodes.
 SET citus.enable_ddl_propagation to DEFAULT;
+-- should not propagate the vacuum and analyze
+SELECT run_command_on_workers($$SELECT wait_for_stats()$$);
+ run_command_on_workers 
+------------------------
+ (localhost,57637,t,"")
+ (localhost,57638,t,"")
+(2 rows)
+
+SELECT run_command_on_workers($$SELECT pg_stat_get_vacuum_count(tablename::regclass) from pg_tables where tablename LIKE 'dustbunnies_%' limit 1$$);
+ run_command_on_workers 
+------------------------
+ (localhost,57637,t,3)
+ (localhost,57638,t,3)
+(2 rows)
+
+SELECT run_command_on_workers($$SELECT pg_stat_get_analyze_count(tablename::regclass) from pg_tables where tablename LIKE 'dustbunnies_%' limit 1$$);
+ run_command_on_workers 
+------------------------
+ (localhost,57637,t,3)
+ (localhost,57638,t,3)
+(2 rows)
+
 -- test worker_hash
 SELECT worker_hash(123);
  worker_hash 

src/test/regress/sql/multi_utilities.sql

@@ -145,6 +145,46 @@ begin
 end
 $$ language plpgsql;
 
+\c - - - :worker_2_port
+CREATE MATERIALIZED VIEW prevcounts AS
+SELECT analyze_count, vacuum_count FROM pg_stat_user_tables
+WHERE relname='dustbunnies_990001';
+-- create function that sleeps until those counters increment
+create function wait_for_stats() returns void as $$
+declare
+  start_time timestamptz := clock_timestamp();
+  analyze_updated bool;
+  vacuum_updated bool;
+begin
+  -- we don't want to wait forever; loop will exit after 10 seconds
+  for i in 1 .. 100 loop
+
+    -- check to see if analyze has been updated
+    SELECT (st.analyze_count >= pc.analyze_count + 1) INTO analyze_updated
+      FROM pg_stat_user_tables AS st, pg_class AS cl, prevcounts AS pc
+     WHERE st.relname='dustbunnies_990001' AND cl.relname='dustbunnies_990001';
+
+     -- check to see if vacuum has been updated
+    SELECT (st.vacuum_count >= pc.vacuum_count + 1) INTO vacuum_updated
+      FROM pg_stat_user_tables AS st, pg_class AS cl, prevcounts AS pc
+     WHERE st.relname='dustbunnies_990001' AND cl.relname='dustbunnies_990001';
+
+    exit when analyze_updated or vacuum_updated;
+
+    -- wait a little
+    perform pg_sleep(0.1);
+
+    -- reset stats snapshot so we can test again
+    perform pg_stat_clear_snapshot();
+
+  end loop;
+
+  -- report time waited in postmaster log (where it won't change test output)
+  raise log 'wait_for_stats delayed % seconds',
+    extract(epoch from clock_timestamp() - start_time);
+end
+$$ language plpgsql;
+
 -- run VACUUM and ANALYZE against the table on the master
 \c - - - :master_port
 VACUUM dustbunnies;
@@ -209,11 +249,22 @@ WHERE tablename = 'dustbunnies_990002' ORDER BY attname;
 -- verify warning for unqualified VACUUM
 VACUUM;
 
+-- check the current number of vacuum and analyze run on dustbunnies
+SELECT run_command_on_workers($$SELECT wait_for_stats()$$);
+SELECT run_command_on_workers($$SELECT pg_stat_get_vacuum_count(tablename::regclass) from pg_tables where tablename LIKE 'dustbunnies_%' limit 1$$);
+SELECT run_command_on_workers($$SELECT pg_stat_get_analyze_count(tablename::regclass) from pg_tables where tablename LIKE 'dustbunnies_%' limit 1$$);
+
 -- and warning when using targeted VACUUM without DDL propagation
 SET citus.enable_ddl_propagation to false;
 VACUUM dustbunnies;
+ANALYZE dustbunnies;
 SET citus.enable_ddl_propagation to DEFAULT;
 
+-- should not propagate the vacuum and analyze
+SELECT run_command_on_workers($$SELECT wait_for_stats()$$);
+SELECT run_command_on_workers($$SELECT pg_stat_get_vacuum_count(tablename::regclass) from pg_tables where tablename LIKE 'dustbunnies_%' limit 1$$);
+SELECT run_command_on_workers($$SELECT pg_stat_get_analyze_count(tablename::regclass) from pg_tables where tablename LIKE 'dustbunnies_%' limit 1$$);
+
 -- test worker_hash
 SELECT worker_hash(123);
 SELECT worker_hash('1997-08-08'::date);