invalidate plan cache in master_update_node (#3758)

* invalidate plan cache in master_update_node

If a plan is cached by postgres but a user uses master_update_node, then
when the plan cache is used for the updated node, they will get the old
nodename/nodepost in the plan. This is because the plan cache doesn't
know about the master_update_node. This could be a problem in prepared
statements or anything that goes into plancache. As a solution the plan
cache is invalidated inside master_update_node.

* add invalidate_inactive_shared_connections test function

We introduce invalidate_inactive_shared_connections udf to be used in
testing. It is possible that a connection count for an inactive node
will be greater than 0 and in that case it will not be removed at the
time of invalidation. However, later we don't have a mechanism to remove
it, which means that it will stay in the hash. For this not to cause a
problem, we use this udf in testing.

* move invalidate_inactive_shared_connections to udfs from test as it will be used in mx

* remove the test udf

* remove the IsInactive check

src/backend/distributed/connection/shared_connection_stats.c

@@ -110,7 +110,6 @@ static uint32 SharedConnectionHashHash(const void *key, Size keysize);
 
 PG_FUNCTION_INFO_V1(citus_remote_connection_stats);
 
-
 /*
  * citus_remote_connection_stats returns all the avaliable information about all
  * the remote connections (a.k.a., connections to remote nodes).

src/backend/distributed/metadata/node_metadata.c

@@ -7,6 +7,7 @@
 #include "postgres.h"
 #include "miscadmin.h"
 #include "funcapi.h"
+#include "utils/plancache.h"
 
 
 #include "access/genam.h"
@@ -740,6 +741,12 @@ master_update_node(PG_FUNCTION_ARGS)
 		LockShardsInPlacementListMetadata(placementList, AccessExclusiveLock);
 	}
 
+	/*
+	 * if we have planned statements such as prepared statements, we should clear the cache so that
+	 * the planned cache doesn't return the old nodename/nodepost.
+	 */
+	ResetPlanCache();
+
 	UpdateNodeLocation(nodeId, newNodeNameString, newNodePort);
 
 	/* we should be able to find the new node from the metadata */

src/backend/distributed/sql/citus--9.2-4--9.3-2.sql

@@ -8,7 +8,6 @@
 #include "udfs/citus_remote_connection_stats/9.3-2.sql"
 #include "udfs/worker_create_or_alter_role/9.3-2.sql"
 #include "udfs/truncate_local_data_after_distributing_table/9.3-2.sql"
-
 -- add citus extension owner as a distributed object, if not already in there
 INSERT INTO citus.pg_dist_object SELECT
   (SELECT oid FROM pg_class WHERE relname = 'pg_authid') AS oid,

src/test/regress/expected/multi_mx_node_metadata.out

@@ -311,6 +311,92 @@ SELECT verify_metadata('localhost', :worker_1_port),
  t               | t
 (1 row)
 
+---------------------------------------------------------------------
+-- Test that master_update_node invalidates the plan cache
+---------------------------------------------------------------------
+PREPARE foo AS SELECT COUNT(*) FROM dist_table_1 WHERE a = 1;
+SET citus.log_remote_commands = ON;
+-- trigger caching for prepared statements
+EXECUTE foo;
+NOTICE:  issuing SELECT count(*) AS count FROM public.dist_table_1_102010 dist_table_1 WHERE (a OPERATOR(pg_catalog.=) 1)
+DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
+ count
+---------------------------------------------------------------------
+     0
+(1 row)
+
+EXECUTE foo;
+NOTICE:  issuing SELECT count(*) AS count FROM public.dist_table_1_102010 dist_table_1 WHERE (a OPERATOR(pg_catalog.=) 1)
+DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
+ count
+---------------------------------------------------------------------
+     0
+(1 row)
+
+EXECUTE foo;
+NOTICE:  issuing SELECT count(*) AS count FROM public.dist_table_1_102010 dist_table_1 WHERE (a OPERATOR(pg_catalog.=) 1)
+DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
+ count
+---------------------------------------------------------------------
+     0
+(1 row)
+
+EXECUTE foo;
+NOTICE:  issuing SELECT count(*) AS count FROM public.dist_table_1_102010 dist_table_1 WHERE (a OPERATOR(pg_catalog.=) 1)
+DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
+ count
+---------------------------------------------------------------------
+     0
+(1 row)
+
+EXECUTE foo;
+NOTICE:  issuing SELECT count(*) AS count FROM public.dist_table_1_102010 dist_table_1 WHERE (a OPERATOR(pg_catalog.=) 1)
+DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
+ count
+---------------------------------------------------------------------
+     0
+(1 row)
+
+EXECUTE foo;
+NOTICE:  issuing SELECT count(*) AS count FROM public.dist_table_1_102010 dist_table_1 WHERE (a OPERATOR(pg_catalog.=) 1)
+DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
+ count
+---------------------------------------------------------------------
+     0
+(1 row)
+
+EXECUTE foo;
+NOTICE:  issuing SELECT count(*) AS count FROM public.dist_table_1_102010 dist_table_1 WHERE (a OPERATOR(pg_catalog.=) 1)
+DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
+ count
+---------------------------------------------------------------------
+     0
+(1 row)
+
+SELECT master_update_node(:nodeid_1, '127.0.0.1', :worker_1_port);
+ master_update_node
+---------------------------------------------------------------------
+
+(1 row)
+
+SELECT wait_until_metadata_sync(30000);
+NOTICE:  issuing LISTEN metadata_sync
+DETAIL:  on server postgres@localhost:xxxxx connectionId: xxxxxxx
+ wait_until_metadata_sync
+---------------------------------------------------------------------
+
+(1 row)
+
+-- make sure the nodename changed.
+EXECUTE foo;
+NOTICE:  issuing SELECT count(*) AS count FROM public.dist_table_1_102010 dist_table_1 WHERE (a OPERATOR(pg_catalog.=) 1)
+DETAIL:  on server postgres@127.0.0.1:57637 connectionId: xxxxxxx
+ count
+---------------------------------------------------------------------
+     0
+(1 row)
+
+SET citus.log_remote_commands TO OFF;
 ---------------------------------------------------------------------
 -- Test that master_update_node can appear in a prepared transaction.
 ---------------------------------------------------------------------

src/test/regress/sql/multi_mx_node_metadata.sql

@@ -139,6 +139,30 @@ ROLLBACK;
 SELECT verify_metadata('localhost', :worker_1_port),
        verify_metadata('localhost', :worker_2_port);
 
+--------------------------------------------------------------------------
+-- Test that master_update_node invalidates the plan cache
+--------------------------------------------------------------------------
+
+PREPARE foo AS SELECT COUNT(*) FROM dist_table_1 WHERE a = 1;
+
+SET citus.log_remote_commands = ON;
+-- trigger caching for prepared statements
+EXECUTE foo;
+EXECUTE foo;
+EXECUTE foo;
+EXECUTE foo;
+EXECUTE foo;
+EXECUTE foo;
+EXECUTE foo;
+
+SELECT master_update_node(:nodeid_1, '127.0.0.1', :worker_1_port);
+SELECT wait_until_metadata_sync(30000);
+
+-- make sure the nodename changed.
+EXECUTE foo;
+
+SET citus.log_remote_commands TO OFF;
+
 --------------------------------------------------------------------------
 -- Test that master_update_node can appear in a prepared transaction.
 --------------------------------------------------------------------------