From 33913bed37796d1ba659cf95988b2b3838b11d9e Mon Sep 17 00:00:00 2001 From: Jelte Fennema Date: Fri, 26 Aug 2022 12:01:36 +0200 Subject: [PATCH] Fix flakyness in failure_connection_establishment (#6251) In CI sometimes failure_connection_establishment would fail with the following error: ```diff -- cancel all connections to this node SELECT citus.mitmproxy('conn.onAuthenticationOk().cancel(' || pg_backend_pid() || ')'); - mitmproxy ---------------------------------------------------------------------- - -(1 row) - +ERROR: canceling statement due to user request +CONTEXT: COPY mitmproxy_result, line 1: "" +SQL statement "COPY mitmproxy_result FROM '/home/circleci/project/src/test/regress/tmp_check/mitmproxy.fifo'" +PL/pgSQL function citus.mitmproxy(text) line 11 at EXECUTE SELECT * FROM citus_check_cluster_node_health(); ``` The reason for this is that the mitm command that was used is very broad and doesn't actually do what the comment says. What happens is that if any connection is made, the current backend is cancelled, which is not the always the same as the backend that made the connection. My assessment is that likely the maintenance daemon makes a connection to the node while we are executing the mitmproxy command. The mitmproxy command goes through, and then triggers a cancel of itself due to the connection made by the maintenance daemon. This PR simply removes this test, since it doesn't seem to test what it intended to test anyway. There's also still the "kill" version of this test, which does do the intended thing. So I don't think we lose important coverage by removing this test. (cherry picked from commit 2a0c0b3ba67564b4747b087df46b4cb31ed31d5c) --- .../expected/failure_connection_establishment.out | 9 --------- .../regress/sql/failure_connection_establishment.sql | 4 ---- 2 files changed, 13 deletions(-) diff --git a/src/test/regress/expected/failure_connection_establishment.out b/src/test/regress/expected/failure_connection_establishment.out index d30e390e3..d032755dd 100644 --- a/src/test/regress/expected/failure_connection_establishment.out +++ b/src/test/regress/expected/failure_connection_establishment.out @@ -396,15 +396,6 @@ SELECT * FROM citus_check_cluster_node_health(); localhost | 57637 | localhost | 57637 | t (4 rows) --- cancel all connections to this node -SELECT citus.mitmproxy('conn.onAuthenticationOk().cancel(' || pg_backend_pid() || ')'); - mitmproxy ---------------------------------------------------------------------- - -(1 row) - -SELECT * FROM citus_check_cluster_node_health(); -ERROR: canceling statement due to user request -- kill connection checks to this node SELECT citus.mitmproxy('conn.onQuery(query="^SELECT 1$").kill()'); mitmproxy diff --git a/src/test/regress/sql/failure_connection_establishment.sql b/src/test/regress/sql/failure_connection_establishment.sql index 1817e3199..5f364cacc 100644 --- a/src/test/regress/sql/failure_connection_establishment.sql +++ b/src/test/regress/sql/failure_connection_establishment.sql @@ -181,10 +181,6 @@ SELECT * FROM citus_check_cluster_node_health(); SELECT citus.mitmproxy('conn.onAuthenticationOk().kill()'); SELECT * FROM citus_check_cluster_node_health(); --- cancel all connections to this node -SELECT citus.mitmproxy('conn.onAuthenticationOk().cancel(' || pg_backend_pid() || ')'); -SELECT * FROM citus_check_cluster_node_health(); - -- kill connection checks to this node SELECT citus.mitmproxy('conn.onQuery(query="^SELECT 1$").kill()'); SELECT * FROM citus_check_cluster_node_health();