From 97da44558b48640ce74bd06039457bab6316f9c3 Mon Sep 17 00:00:00 2001 From: Nils Dijk Date: Tue, 13 Nov 2018 16:10:36 +0100 Subject: [PATCH] Description: Fix failures of tests on recent postgres builds In recent postgres builds you cannot set client_min_messages to values higher then ERROR, if will silently set it to ERROR if so. During some tests we would set it to fatal to hide random values (eg. pid's of processes) from the test output. This patch will use different tactics for hiding these values. --- src/test/regress/.gitignore | 3 +++ .../regress/expected/failure_multi_dml.out | 27 ++++++++++++------- .../expected/failure_real_time_select.out | 22 ++++----------- .../expected/multi_task_string_size.out | 6 ++++- .../regress/expected/multi_test_helpers.out | 10 +++++++ src/test/regress/sql/failure_multi_dml.sql | 25 +++++++++-------- .../regress/sql/failure_real_time_select.sql | 15 ++--------- .../regress/sql/multi_task_string_size.sql | 4 ++- src/test/regress/sql/multi_test_helpers.sql | 11 ++++++++ 9 files changed, 69 insertions(+), 54 deletions(-) diff --git a/src/test/regress/.gitignore b/src/test/regress/.gitignore index 477bd27cc..6e3048cce 100644 --- a/src/test/regress/.gitignore +++ b/src/test/regress/.gitignore @@ -9,3 +9,6 @@ # Regression test output /regression.diffs /regression.out + +# Failure test side effets +/proxy.output diff --git a/src/test/regress/expected/failure_multi_dml.out b/src/test/regress/expected/failure_multi_dml.out index 1f1146bd0..380fab673 100644 --- a/src/test/regress/expected/failure_multi_dml.out +++ b/src/test/regress/expected/failure_multi_dml.out @@ -204,17 +204,28 @@ SELECT citus.mitmproxy('conn.onQuery(query="^PREPARE TRANSACTION").kill()'); (1 row) --- hide the error message (it has the PID)... +-- this transaction block will be sent to the coordinator as a remote command to hide the +-- error message that is caused during commit. -- we'll test for the txn side-effects to ensure it didn't run -SET client_min_messages TO FATAL; +SELECT master_run_on_worker( + ARRAY['localhost']::text[], + ARRAY[:master_port]::int[], + ARRAY[' BEGIN; DELETE FROM dml_test WHERE id = 1; DELETE FROM dml_test WHERE id = 2; -INSERT INTO dml_test VALUES (5, 'Epsilon'); -UPDATE dml_test SET name = 'alpha' WHERE id = 1; -UPDATE dml_test SET name = 'gamma' WHERE id = 3; +INSERT INTO dml_test VALUES (5, ''Epsilon''); +UPDATE dml_test SET name = ''alpha'' WHERE id = 1; +UPDATE dml_test SET name = ''gamma'' WHERE id = 3; COMMIT; -SET client_min_messages TO DEFAULT; + '], + false +); + master_run_on_worker +--------------------------- + (localhost,57636,t,BEGIN) +(1 row) + SELECT citus.mitmproxy('conn.allow()'); mitmproxy ----------- @@ -249,9 +260,7 @@ SELECT citus.mitmproxy('conn.onQuery(query="^PREPARE TRANSACTION").cancel(' || (1 row) --- hide the error message (it has the PID)... -- we'll test for the txn side-effects to ensure it didn't run -SET client_min_messages TO FATAL; BEGIN; DELETE FROM dml_test WHERE id = 1; DELETE FROM dml_test WHERE id = 2; @@ -259,7 +268,7 @@ INSERT INTO dml_test VALUES (5, 'Epsilon'); UPDATE dml_test SET name = 'alpha' WHERE id = 1; UPDATE dml_test SET name = 'gamma' WHERE id = 3; COMMIT; -SET client_min_messages TO DEFAULT; +ERROR: canceling statement due to user request SELECT citus.mitmproxy('conn.allow()'); mitmproxy ----------- diff --git a/src/test/regress/expected/failure_real_time_select.out b/src/test/regress/expected/failure_real_time_select.out index da4d77bac..967569af2 100644 --- a/src/test/regress/expected/failure_real_time_select.out +++ b/src/test/regress/expected/failure_real_time_select.out @@ -23,16 +23,6 @@ SELECT create_distributed_table('test_table','id'); -- Populate data to the table INSERT INTO test_table VALUES(1,1,1),(1,2,2),(2,1,1),(2,2,2),(3,1,1),(3,2,2); --- Create a function to make sure that queries returning the same result -CREATE FUNCTION raise_failed_execution(query text) RETURNS void AS $$ -BEGIN - EXECUTE query; - EXCEPTION WHEN OTHERS THEN - IF SQLERRM LIKE 'failed to execute task%' THEN - RAISE 'Task failed to execute'; - END IF; -END; -$$LANGUAGE plpgsql; -- Kill when the first COPY command arrived, since we have a single placement -- it is expected to error out. SET client_min_messages TO ERROR; @@ -42,9 +32,9 @@ SELECT citus.mitmproxy('conn.onQuery(query="^COPY").kill()'); (1 row) -SELECT raise_failed_execution('SELECT count(*) FROM test_table'); +SELECT public.raise_failed_execution('SELECT count(*) FROM test_table'); ERROR: Task failed to execute -CONTEXT: PL/pgSQL function raise_failed_execution(text) line 6 at RAISE +CONTEXT: PL/pgSQL function public.raise_failed_execution(text) line 6 at RAISE SET client_min_messages TO DEFAULT; -- Kill the connection with a CTE SELECT citus.mitmproxy('conn.onQuery(query="^COPY").kill()'); @@ -70,12 +60,12 @@ SELECT citus.mitmproxy('conn.onQuery(query="^COPY").after(1).kill()'); (1 row) -SELECT raise_failed_execution('WITH +SELECT public.raise_failed_execution('WITH results AS (SELECT * FROM test_table) SELECT * FROM test_table, results WHERE test_table.id = results.id'); ERROR: Task failed to execute -CONTEXT: PL/pgSQL function raise_failed_execution(text) line 6 at RAISE +CONTEXT: PL/pgSQL function public.raise_failed_execution(text) line 6 at RAISE SET client_min_messages TO DEFAULT; -- In parallel execution mode Citus opens separate connections for each shard -- so killing the connection after the first copy does not break it. @@ -297,7 +287,5 @@ WARNING: could not consume data from worker node COMMIT; DROP SCHEMA real_time_select_failure CASCADE; -NOTICE: drop cascades to 2 other objects -DETAIL: drop cascades to function raise_failed_execution(text) -drop cascades to table test_table +NOTICE: drop cascades to table test_table SET search_path TO default; diff --git a/src/test/regress/expected/multi_task_string_size.out b/src/test/regress/expected/multi_task_string_size.out index 57d527c07..7660e4329 100644 --- a/src/test/regress/expected/multi_task_string_size.out +++ b/src/test/regress/expected/multi_task_string_size.out @@ -225,8 +225,12 @@ ERROR: parameter "citus.max_task_string_size" cannot be changed without restart -- error message may vary between executions -- hiding warning and error message -- no output means the query has failed -SET client_min_messages to FATAL; +SET client_min_messages to ERROR; +SELECT raise_failed_execution(' SELECT u.* FROM wide_table u JOIN wide_table v ON (u.long_column_002 = v.long_column_003); +'); +ERROR: Task failed to execute +CONTEXT: PL/pgSQL function raise_failed_execution(text) line 6 at RAISE -- following will succeed since it fetches few columns SELECT u.long_column_001, u.long_column_002, u.long_column_003 FROM wide_table u JOIN wide_table v ON (u.long_column_002 = v.long_column_003); long_column_001 | long_column_002 | long_column_003 diff --git a/src/test/regress/expected/multi_test_helpers.out b/src/test/regress/expected/multi_test_helpers.out index 6ffe7eae8..40073f61e 100644 --- a/src/test/regress/expected/multi_test_helpers.out +++ b/src/test/regress/expected/multi_test_helpers.out @@ -109,3 +109,13 @@ $desc_views$ (1 row) +-- Create a function to make sure that queries returning the same result +CREATE FUNCTION raise_failed_execution(query text) RETURNS void AS $$ +BEGIN + EXECUTE query; + EXCEPTION WHEN OTHERS THEN + IF SQLERRM LIKE 'failed to execute task%' THEN + RAISE 'Task failed to execute'; + END IF; +END; +$$LANGUAGE plpgsql; diff --git a/src/test/regress/sql/failure_multi_dml.sql b/src/test/regress/sql/failure_multi_dml.sql index 49e2f1fbe..a317da996 100644 --- a/src/test/regress/sql/failure_multi_dml.sql +++ b/src/test/regress/sql/failure_multi_dml.sql @@ -107,19 +107,23 @@ SELECT * FROM dml_test ORDER BY id ASC; -- fail at PREPARE TRANSACTION SELECT citus.mitmproxy('conn.onQuery(query="^PREPARE TRANSACTION").kill()'); --- hide the error message (it has the PID)... +-- this transaction block will be sent to the coordinator as a remote command to hide the +-- error message that is caused during commit. -- we'll test for the txn side-effects to ensure it didn't run -SET client_min_messages TO FATAL; - +SELECT master_run_on_worker( + ARRAY['localhost']::text[], + ARRAY[:master_port]::int[], + ARRAY[' BEGIN; DELETE FROM dml_test WHERE id = 1; DELETE FROM dml_test WHERE id = 2; -INSERT INTO dml_test VALUES (5, 'Epsilon'); -UPDATE dml_test SET name = 'alpha' WHERE id = 1; -UPDATE dml_test SET name = 'gamma' WHERE id = 3; +INSERT INTO dml_test VALUES (5, ''Epsilon''); +UPDATE dml_test SET name = ''alpha'' WHERE id = 1; +UPDATE dml_test SET name = ''gamma'' WHERE id = 3; COMMIT; - -SET client_min_messages TO DEFAULT; + '], + false +); SELECT citus.mitmproxy('conn.allow()'); SELECT shardid FROM pg_dist_shard_placement WHERE shardstate = 3; @@ -132,10 +136,7 @@ SELECT * FROM dml_test ORDER BY id ASC; -- cancel at PREPARE TRANSACTION SELECT citus.mitmproxy('conn.onQuery(query="^PREPARE TRANSACTION").cancel(' || pg_backend_pid() || ')'); --- hide the error message (it has the PID)... -- we'll test for the txn side-effects to ensure it didn't run -SET client_min_messages TO FATAL; - BEGIN; DELETE FROM dml_test WHERE id = 1; DELETE FROM dml_test WHERE id = 2; @@ -144,8 +145,6 @@ UPDATE dml_test SET name = 'alpha' WHERE id = 1; UPDATE dml_test SET name = 'gamma' WHERE id = 3; COMMIT; -SET client_min_messages TO DEFAULT; - SELECT citus.mitmproxy('conn.allow()'); SELECT shardid FROM pg_dist_shard_placement WHERE shardstate = 3; SELECT recover_prepared_transactions(); diff --git a/src/test/regress/sql/failure_real_time_select.sql b/src/test/regress/sql/failure_real_time_select.sql index a9df56c80..3dacca315 100644 --- a/src/test/regress/sql/failure_real_time_select.sql +++ b/src/test/regress/sql/failure_real_time_select.sql @@ -17,22 +17,11 @@ SELECT create_distributed_table('test_table','id'); -- Populate data to the table INSERT INTO test_table VALUES(1,1,1),(1,2,2),(2,1,1),(2,2,2),(3,1,1),(3,2,2); --- Create a function to make sure that queries returning the same result -CREATE FUNCTION raise_failed_execution(query text) RETURNS void AS $$ -BEGIN - EXECUTE query; - EXCEPTION WHEN OTHERS THEN - IF SQLERRM LIKE 'failed to execute task%' THEN - RAISE 'Task failed to execute'; - END IF; -END; -$$LANGUAGE plpgsql; - -- Kill when the first COPY command arrived, since we have a single placement -- it is expected to error out. SET client_min_messages TO ERROR; SELECT citus.mitmproxy('conn.onQuery(query="^COPY").kill()'); -SELECT raise_failed_execution('SELECT count(*) FROM test_table'); +SELECT public.raise_failed_execution('SELECT count(*) FROM test_table'); SET client_min_messages TO DEFAULT; -- Kill the connection with a CTE @@ -46,7 +35,7 @@ WHERE test_table.id = results.id; -- killing connection after first successful query should break. SET client_min_messages TO ERROR; SELECT citus.mitmproxy('conn.onQuery(query="^COPY").after(1).kill()'); -SELECT raise_failed_execution('WITH +SELECT public.raise_failed_execution('WITH results AS (SELECT * FROM test_table) SELECT * FROM test_table, results WHERE test_table.id = results.id'); diff --git a/src/test/regress/sql/multi_task_string_size.sql b/src/test/regress/sql/multi_task_string_size.sql index c650902a0..a1c104b6e 100644 --- a/src/test/regress/sql/multi_task_string_size.sql +++ b/src/test/regress/sql/multi_task_string_size.sql @@ -220,9 +220,11 @@ SET citus.max_task_string_size TO 20000; -- error message may vary between executions -- hiding warning and error message -- no output means the query has failed -SET client_min_messages to FATAL; +SET client_min_messages to ERROR; +SELECT raise_failed_execution(' SELECT u.* FROM wide_table u JOIN wide_table v ON (u.long_column_002 = v.long_column_003); +'); -- following will succeed since it fetches few columns SELECT u.long_column_001, u.long_column_002, u.long_column_003 FROM wide_table u JOIN wide_table v ON (u.long_column_002 = v.long_column_003); diff --git a/src/test/regress/sql/multi_test_helpers.sql b/src/test/regress/sql/multi_test_helpers.sql index 23bbd97f4..26bb17882 100644 --- a/src/test/regress/sql/multi_test_helpers.sql +++ b/src/test/regress/sql/multi_test_helpers.sql @@ -106,3 +106,14 @@ ORDER BY a.attrelid, a.attnum; $desc_views$ ); + +-- Create a function to make sure that queries returning the same result +CREATE FUNCTION raise_failed_execution(query text) RETURNS void AS $$ +BEGIN + EXECUTE query; + EXCEPTION WHEN OTHERS THEN + IF SQLERRM LIKE 'failed to execute task%' THEN + RAISE 'Task failed to execute'; + END IF; +END; +$$LANGUAGE plpgsql;