From 4bde83e1d2faed485ecb697edd7fddaa15cf25d2 Mon Sep 17 00:00:00 2001 From: Marco Slot Date: Tue, 24 Oct 2017 14:21:42 +0200 Subject: [PATCH] Relay error message if DML fails on worker --- .../executor/multi_router_executor.c | 23 ++++++++++++++++- .../regress/expected/multi_insert_select.out | 3 +-- .../regress/expected/multi_modifications.out | 6 ++--- .../expected/multi_modifying_xacts.out | 25 +++++++++++++------ .../expected/multi_mx_modifications.out | 3 ++- .../expected/multi_mx_modifying_xacts.out | 12 +++------ .../regress/sql/multi_modifying_xacts.sql | 15 ++++++++--- 7 files changed, 62 insertions(+), 25 deletions(-) diff --git a/src/backend/distributed/executor/multi_router_executor.c b/src/backend/distributed/executor/multi_router_executor.c index 70f13c82a..8aeae4648 100644 --- a/src/backend/distributed/executor/multi_router_executor.c +++ b/src/backend/distributed/executor/multi_router_executor.c @@ -701,6 +701,7 @@ ExecuteSingleModifyTask(CitusScanState *scanState, Task *task, bool multipleTask ListCell *taskPlacementCell = NULL; ListCell *connectionCell = NULL; int64 affectedTupleCount = -1; + int failureCount = 0; bool resultsOK = false; bool gotResults = false; @@ -761,12 +762,14 @@ ExecuteSingleModifyTask(CitusScanState *scanState, Task *task, bool multipleTask * MarkFailedShardPlacements() to ensure future statements will not use this * placement. */ + failureCount++; continue; } queryOK = SendQueryInSingleRowMode(connection, queryString, paramListInfo); if (!queryOK) { + failureCount++; continue; } @@ -782,6 +785,15 @@ ExecuteSingleModifyTask(CitusScanState *scanState, Task *task, bool multipleTask failOnError = true; } + if (failureCount + 1 == list_length(taskPlacementList)) + { + /* + * If we already failed on all other placements (possibly 0), + * relay errors directly. + */ + failOnError = true; + } + /* * If caller is interested, store query results the first time * through. The output of the query's execution on other shards is @@ -818,9 +830,18 @@ ExecuteSingleModifyTask(CitusScanState *scanState, Task *task, bool multipleTask resultsOK = true; gotResults = true; } + else + { + failureCount++; + } } - /* if all placements failed, error out */ + /* + * If a command results in an error on all workers, we relay the last error + * in the loop above by setting failOnError. However, if all connections fail + * we still complete the loop without throwing an error. In that case, throw + * an error below. + */ if (!resultsOK) { ereport(ERROR, (errmsg("could not modify any active placements"))); diff --git a/src/test/regress/expected/multi_insert_select.out b/src/test/regress/expected/multi_insert_select.out index 9c1bbfc49..05752141c 100644 --- a/src/test/regress/expected/multi_insert_select.out +++ b/src/test/regress/expected/multi_insert_select.out @@ -139,8 +139,7 @@ FROM WHERE user_id = 0; WARNING: function public.evaluate_on_master(integer) does not exist -WARNING: function public.evaluate_on_master(integer) does not exist -ERROR: could not modify any active placements +ERROR: function public.evaluate_on_master(integer) does not exist \set VERBOSITY default -- add one more row INSERT INTO raw_events_first (user_id, time) VALUES diff --git a/src/test/regress/expected/multi_modifications.out b/src/test/regress/expected/multi_modifications.out index a4a00ed67..64ab1bf42 100644 --- a/src/test/regress/expected/multi_modifications.out +++ b/src/test/regress/expected/multi_modifications.out @@ -188,7 +188,8 @@ DETAIL: Key (id)=(32743) already exists. CONTEXT: while executing command on localhost:57638 -- INSERT, with RETURNING specified, failing with a non-constraint error INSERT INTO limit_orders VALUES (34153, 'LEE', 5994, '2001-04-16 03:37:28', 'buy', 0.58) RETURNING id / 0; -ERROR: could not modify any active placements +ERROR: division by zero +CONTEXT: while executing command on localhost:57637 SET client_min_messages TO DEFAULT; -- commands with non-constant partition values are supported INSERT INTO limit_orders VALUES (random() * 100, 'ORCL', 152, '2011-08-25 11:50:45', @@ -399,9 +400,8 @@ ALTER TABLE limit_orders_750000 RENAME TO renamed_orders; \c - - - :master_port -- Fourth: Perform an INSERT on the remaining node INSERT INTO limit_orders VALUES (276, 'ADR', 140, '2007-07-02 16:32:15', 'sell', 43.67); -WARNING: relation "public.limit_orders_750000" does not exist +ERROR: relation "public.limit_orders_750000" does not exist CONTEXT: while executing command on localhost:57637 -ERROR: could not modify any active placements -- Last: Verify worker is still healthy SELECT count(*) FROM pg_dist_shard_placement AS sp, diff --git a/src/test/regress/expected/multi_modifying_xacts.out b/src/test/regress/expected/multi_modifying_xacts.out index 3838c8d4c..2abe0706a 100644 --- a/src/test/regress/expected/multi_modifying_xacts.out +++ b/src/test/regress/expected/multi_modifying_xacts.out @@ -574,8 +574,7 @@ INSERT INTO objects VALUES (2, 'BAD'); WARNING: illegal value INSERT INTO labs VALUES (8, 'Aperture Science'); INSERT INTO labs VALUES (9, 'BAD'); -WARNING: illegal value -ERROR: could not modify any active placements +ERROR: illegal value COMMIT; -- data should NOT be persisted SELECT * FROM objects WHERE id = 1; @@ -951,8 +950,7 @@ FOR EACH ROW EXECUTE PROCEDURE reject_bad_hash(); BEGIN; INSERT INTO reference_modifying_xacts VALUES (55, 10); INSERT INTO hash_modifying_xacts VALUES (997, 1); -WARNING: illegal value -ERROR: could not modify any active placements +ERROR: illegal value COMMIT; -- ensure that the value didn't go into the reference table SELECT * FROM reference_modifying_xacts WHERE key = 55; @@ -1305,11 +1303,24 @@ WARNING: connection error: localhost:57637 2 (1 row) --- connect back to the worker and set rename the test_user back -\c - :default_user - :worker_1_port -ALTER USER test_user_new RENAME TO test_user; +-- break the other node as well +\c - :default_user - :worker_2_port +ALTER USER test_user RENAME TO test_user_new; +\c - test_user - :master_port +-- fails on all shard placements +INSERT INTO numbers_hash_failure_test VALUES (2,2); +WARNING: connection error: localhost:57638 +ERROR: could not modify any active placements -- connect back to the master with the proper user to continue the tests \c - :default_user - :master_port +-- unbreak both nodes by renaming the user back to the original name +SELECT * FROM run_command_on_workers('ALTER USER test_user_new RENAME TO test_user'); + nodename | nodeport | success | result +-----------+----------+---------+------------ + localhost | 57637 | t | ALTER ROLE + localhost | 57638 | t | ALTER ROLE +(2 rows) + DROP TABLE reference_modifying_xacts, hash_modifying_xacts, hash_modifying_xacts_second, reference_failure_test, numbers_hash_failure_test; SELECT * FROM run_command_on_workers('DROP USER test_user'); diff --git a/src/test/regress/expected/multi_mx_modifications.out b/src/test/regress/expected/multi_mx_modifications.out index b9f0b09b6..fbd0bba89 100644 --- a/src/test/regress/expected/multi_mx_modifications.out +++ b/src/test/regress/expected/multi_mx_modifications.out @@ -91,7 +91,8 @@ DETAIL: Key (id)=(32743) already exists. CONTEXT: while executing command on localhost:57638 -- INSERT, with RETURNING specified, failing with a non-constraint error INSERT INTO limit_orders_mx VALUES (34153, 'LEE', 5994, '2001-04-16 03:37:28', 'buy', 0.58) RETURNING id / 0; -ERROR: could not modify any active placements +ERROR: division by zero +CONTEXT: while executing command on localhost:57638 SET client_min_messages TO DEFAULT; -- commands with non-constant partition values are unsupported INSERT INTO limit_orders_mx VALUES (random() * 100, 'ORCL', 152, '2011-08-25 11:50:45', diff --git a/src/test/regress/expected/multi_mx_modifying_xacts.out b/src/test/regress/expected/multi_mx_modifying_xacts.out index 0efee2b2a..3bec55391 100644 --- a/src/test/regress/expected/multi_mx_modifying_xacts.out +++ b/src/test/regress/expected/multi_mx_modifying_xacts.out @@ -242,8 +242,7 @@ FOR EACH ROW EXECUTE PROCEDURE reject_bad_mx(); BEGIN; INSERT INTO labs_mx VALUES (7, 'E Corp'); INSERT INTO objects_mx VALUES (2, 'BAD'); -WARNING: illegal value -ERROR: could not modify any active placements +ERROR: illegal value COMMIT; -- data should NOT be persisted SELECT * FROM objects_mx WHERE id = 2; @@ -262,8 +261,7 @@ SELECT * FROM labs_mx WHERE id = 7; BEGIN; INSERT INTO labs_mx VALUES (7, 'E Corp'); INSERT INTO objects_mx VALUES (2, 'BAD'); -WARNING: illegal value -ERROR: could not modify any active placements +ERROR: illegal value COMMIT; -- data should NOT be persisted SELECT * FROM objects_mx WHERE id = 2; @@ -286,8 +284,7 @@ FOR EACH ROW EXECUTE PROCEDURE reject_bad_mx(); BEGIN; INSERT INTO objects_mx VALUES (1, 'apple'); INSERT INTO objects_mx VALUES (2, 'BAD'); -WARNING: illegal value -ERROR: could not modify any active placements +ERROR: illegal value INSERT INTO labs_mx VALUES (8, 'Aperture Science'); ERROR: current transaction is aborted, commands ignored until end of transaction block INSERT INTO labs_mx VALUES (9, 'BAD'); @@ -309,8 +306,7 @@ SELECT * FROM labs_mx WHERE id = 8; BEGIN; INSERT INTO objects_mx VALUES (1, 'apple'); INSERT INTO objects_mx VALUES (2, 'BAD'); -WARNING: illegal value -ERROR: could not modify any active placements +ERROR: illegal value INSERT INTO labs_mx VALUES (8, 'Aperture Science'); ERROR: current transaction is aborted, commands ignored until end of transaction block INSERT INTO labs_mx VALUES (9, 'BAD'); diff --git a/src/test/regress/sql/multi_modifying_xacts.sql b/src/test/regress/sql/multi_modifying_xacts.sql index 4d90284d6..3b8155813 100644 --- a/src/test/regress/sql/multi_modifying_xacts.sql +++ b/src/test/regress/sql/multi_modifying_xacts.sql @@ -963,12 +963,21 @@ ORDER BY shardid, nodeport; -- verify data is inserted SELECT count(*) FROM numbers_hash_failure_test; --- connect back to the worker and set rename the test_user back -\c - :default_user - :worker_1_port -ALTER USER test_user_new RENAME TO test_user; +-- break the other node as well +\c - :default_user - :worker_2_port +ALTER USER test_user RENAME TO test_user_new; + +\c - test_user - :master_port + +-- fails on all shard placements +INSERT INTO numbers_hash_failure_test VALUES (2,2); -- connect back to the master with the proper user to continue the tests \c - :default_user - :master_port + +-- unbreak both nodes by renaming the user back to the original name +SELECT * FROM run_command_on_workers('ALTER USER test_user_new RENAME TO test_user'); + DROP TABLE reference_modifying_xacts, hash_modifying_xacts, hash_modifying_xacts_second, reference_failure_test, numbers_hash_failure_test;