Skip to content

Commit

Permalink
Merge pull request #50 from jfinzel/squash_kill_changes
Browse files Browse the repository at this point in the history
Undo kill blockers change which is overly aggressive

* https://github.com/enova/pgl_ddl_deploy:
  Undo kill blockers change which is overly aggressive
  • Loading branch information
Jerry Sievers committed Sep 6, 2019
2 parents 5bbf857 + 76fe85a commit bd7c393
Show file tree
Hide file tree
Showing 6 changed files with 3 additions and 111 deletions.
2 changes: 0 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ https://innovation.enova.com/pursuing-postgres-ddl-replication/
### Release 1.6
Summary of changes:
* Workaround pglogical 2.2.2 failure resulting from unstable `debug_query_string` results
* Support killing blockers on child tables while modifying parent
* Support killing blockers involved in fkey relationship
* Add more tags as default for common use cases
* Bug fix: Fix raise message escape % bug
* Bug fix: Only auto-add tables to replication if CREATE TABLE tag configured
Expand Down
47 changes: 0 additions & 47 deletions expected/25_1_5_features.out
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ SELECT * FROM pgl_ddl_deploy.exceptions;

CREATE TABLE public.foo(id serial primary key, bla int);
CREATE TABLE public.foo2 () INHERITS (public.foo);
CREATE TABLE public.foo3 (id serial primary key, foo_id int references public.foo (id));
CREATE TABLE public.bar(id serial primary key, bla int);
\! PGOPTIONS='--client-min-messages=warning' psql -d contrib_regression -c "BEGIN; SELECT * FROM public.foo; SELECT pg_sleep(30);" > /dev/null 2>&1 &
SELECT pg_sleep(1);
Expand Down Expand Up @@ -317,53 +316,7 @@ FROM pgl_ddl_deploy.kill_blockers('terminate','public','foo');
--------+------------+-------+-------+----------+----------
(0 rows)
***/
/*** TEST FKEY RELATED TABLE BLOCKER KILLER ***/
-- Same workflow as above, but instead select from a table which has an fkey reference to foo.id
\! PGOPTIONS='--client-min-messages=warning' psql -d contrib_regression -c "BEGIN; SELECT * FROM public.foo3; SELECT pg_sleep(30);" > /dev/null 2>&1 &
SELECT pg_sleep(1);
pg_sleep
----------

(1 row)

SELECT signal, successful, state, query, reported, pg_sleep(1)
FROM pgl_ddl_deploy.kill_blockers('terminate','public','foo');
signal | successful | state | query | reported | pg_sleep
-----------+------------+--------+--------------------------------------------------------+----------+----------
terminate | t | active | BEGIN; SELECT * FROM public.foo3; SELECT pg_sleep(30); | f |
(1 row)

/*** With <=1.5, it showed this. But it should kill the process.
signal | successful | state | query | reported | pg_sleep
--------+------------+-------+-------+----------+----------
(0 rows)
***/
/*** TEST REFERENCED BY TABLE BLOCKER KILLER ***/
-- Same workflow as above, but instead select from a table which has a pkey (foo) which is referenced by another table being altered (foo3)
\! PGOPTIONS='--client-min-messages=warning' psql -d contrib_regression -c "BEGIN; SELECT * FROM public.foo; SELECT pg_sleep(30);" > /dev/null 2>&1 &
SELECT pg_sleep(1);
pg_sleep
----------

(1 row)

SELECT signal, successful, state, query, reported, pg_sleep(1)
FROM pgl_ddl_deploy.kill_blockers('terminate','public','foo3');
signal | successful | state | query | reported | pg_sleep
-----------+------------+--------+-------------------------------------------------------+----------+----------
terminate | t | active | BEGIN; SELECT * FROM public.foo; SELECT pg_sleep(30); | f |
(1 row)

/*** With <=1.5, it showed this. But it should kill the process.
signal | successful | state | query | reported | pg_sleep
--------+------------+-------+-------+----------+----------
(0 rows)
***/
SET lock_timeout TO 1000;
DROP TABLE public.foo CASCADE;
-- With <=1.5, lock is still in place leading to ERROR: canceling statement due to lock timeout
DROP TABLE public.foo3 CASCADE;
-- With <=1.5, lock is still in place leading to ERROR: canceling statement due to lock timeout
TABLE bar;
id | bla
----+-----
Expand Down
12 changes: 1 addition & 11 deletions functions/kill_blockers.sql
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,6 @@ INNER JOIN pg_stat_activity a on l.pid = a.pid
LEFT JOIN pg_inherits pi ON pi.inhrelid = c.oid
LEFT JOIN pg_class ipc on ipc.oid = pi.inhparent
LEFT JOIN pg_namespace ipn on ipn.oid = ipc.relnamespace
-- We need to check if there is a lock on a table with an fkey reference to this table
LEFT JOIN pg_constraint cr ON cr.conrelid = c.oid
LEFT JOIN pg_class crc ON crc.oid = cr.confrelid
LEFT JOIN pg_namespace crn ON crn.oid = crc.relnamespace
-- We need to check if there is a lock on a table with a pkey that is referenced by this table
LEFT JOIN pg_constraint cf ON cf.confrelid = c.oid
LEFT JOIN pg_class cfc ON cfc.oid = cf.conrelid
LEFT JOIN pg_namespace cfn ON cfn.oid = cfc.relnamespace
-- We do not exclude either postgres user or pglogical processes, because we even want to cancel autovac blocks.
-- It should not be possible to contend with pglogical write processes (at least as of pglogical 2.2), because
-- these run single-threaded using the same process that is doing the DDL and already holds any lock it needs
Expand All @@ -87,9 +79,7 @@ WHERE NOT a.pid = pg_backend_pid()
-- both nspname and relname will be an empty string, thus a no-op, if for some reason one or the other
-- is not found on the provider side in pg_event_trigger_ddl_commands(). This is a safety mechanism!
AND ((n.nspname = p_nspname AND c.relname = p_relname)
OR (ipn.nspname = p_nspname AND ipc.relname = p_relname)
OR (crn.nspname = p_nspname AND crc.relname = p_relname)
OR (cfn.nspname = p_nspname AND cfc.relname = p_relname))
OR (ipn.nspname = p_nspname AND ipc.relname = p_relname))
AND a.datname = current_database()
AND c.relkind = 'r'
AND l.locktype = 'relation'
Expand Down
12 changes: 1 addition & 11 deletions pgl_ddl_deploy--1.5--1.6.sql
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,6 @@ INNER JOIN pg_stat_activity a on l.pid = a.pid
LEFT JOIN pg_inherits pi ON pi.inhrelid = c.oid
LEFT JOIN pg_class ipc on ipc.oid = pi.inhparent
LEFT JOIN pg_namespace ipn on ipn.oid = ipc.relnamespace
-- We need to check if there is a lock on a table with an fkey reference to this table
LEFT JOIN pg_constraint cr ON cr.conrelid = c.oid
LEFT JOIN pg_class crc ON crc.oid = cr.confrelid
LEFT JOIN pg_namespace crn ON crn.oid = crc.relnamespace
-- We need to check if there is a lock on a table with a pkey that is referenced by this table
LEFT JOIN pg_constraint cf ON cf.confrelid = c.oid
LEFT JOIN pg_class cfc ON cfc.oid = cf.conrelid
LEFT JOIN pg_namespace cfn ON cfn.oid = cfc.relnamespace
-- We do not exclude either postgres user or pglogical processes, because we even want to cancel autovac blocks.
-- It should not be possible to contend with pglogical write processes (at least as of pglogical 2.2), because
-- these run single-threaded using the same process that is doing the DDL and already holds any lock it needs
Expand All @@ -116,9 +108,7 @@ WHERE NOT a.pid = pg_backend_pid()
-- both nspname and relname will be an empty string, thus a no-op, if for some reason one or the other
-- is not found on the provider side in pg_event_trigger_ddl_commands(). This is a safety mechanism!
AND ((n.nspname = p_nspname AND c.relname = p_relname)
OR (ipn.nspname = p_nspname AND ipc.relname = p_relname)
OR (crn.nspname = p_nspname AND crc.relname = p_relname)
OR (cfn.nspname = p_nspname AND cfc.relname = p_relname))
OR (ipn.nspname = p_nspname AND ipc.relname = p_relname))
AND a.datname = current_database()
AND c.relkind = 'r'
AND l.locktype = 'relation'
Expand Down
12 changes: 1 addition & 11 deletions pgl_ddl_deploy--1.6.sql
Original file line number Diff line number Diff line change
Expand Up @@ -5938,14 +5938,6 @@ INNER JOIN pg_stat_activity a on l.pid = a.pid
LEFT JOIN pg_inherits pi ON pi.inhrelid = c.oid
LEFT JOIN pg_class ipc on ipc.oid = pi.inhparent
LEFT JOIN pg_namespace ipn on ipn.oid = ipc.relnamespace
-- We need to check if there is a lock on a table with an fkey reference to this table
LEFT JOIN pg_constraint cr ON cr.conrelid = c.oid
LEFT JOIN pg_class crc ON crc.oid = cr.confrelid
LEFT JOIN pg_namespace crn ON crn.oid = crc.relnamespace
-- We need to check if there is a lock on a table with a pkey that is referenced by this table
LEFT JOIN pg_constraint cf ON cf.confrelid = c.oid
LEFT JOIN pg_class cfc ON cfc.oid = cf.conrelid
LEFT JOIN pg_namespace cfn ON cfn.oid = cfc.relnamespace
-- We do not exclude either postgres user or pglogical processes, because we even want to cancel autovac blocks.
-- It should not be possible to contend with pglogical write processes (at least as of pglogical 2.2), because
-- these run single-threaded using the same process that is doing the DDL and already holds any lock it needs
Expand All @@ -5954,9 +5946,7 @@ WHERE NOT a.pid = pg_backend_pid()
-- both nspname and relname will be an empty string, thus a no-op, if for some reason one or the other
-- is not found on the provider side in pg_event_trigger_ddl_commands(). This is a safety mechanism!
AND ((n.nspname = p_nspname AND c.relname = p_relname)
OR (ipn.nspname = p_nspname AND ipc.relname = p_relname)
OR (crn.nspname = p_nspname AND crc.relname = p_relname)
OR (cfn.nspname = p_nspname AND cfc.relname = p_relname))
OR (ipn.nspname = p_nspname AND ipc.relname = p_relname))
AND a.datname = current_database()
AND c.relkind = 'r'
AND l.locktype = 'relation'
Expand Down
29 changes: 0 additions & 29 deletions sql/25_1_5_features.sql
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ SELECT * FROM pgl_ddl_deploy.exceptions;

CREATE TABLE public.foo(id serial primary key, bla int);
CREATE TABLE public.foo2 () INHERITS (public.foo);
CREATE TABLE public.foo3 (id serial primary key, foo_id int references public.foo (id));
CREATE TABLE public.bar(id serial primary key, bla int);
\! PGOPTIONS='--client-min-messages=warning' psql -d contrib_regression -c "BEGIN; SELECT * FROM public.foo; SELECT pg_sleep(30);" > /dev/null 2>&1 &
SELECT pg_sleep(1);
Expand Down Expand Up @@ -187,35 +186,7 @@ FROM pgl_ddl_deploy.kill_blockers('terminate','public','foo');
(0 rows)
***/

/*** TEST FKEY RELATED TABLE BLOCKER KILLER ***/
-- Same workflow as above, but instead select from a table which has an fkey reference to foo.id
\! PGOPTIONS='--client-min-messages=warning' psql -d contrib_regression -c "BEGIN; SELECT * FROM public.foo3; SELECT pg_sleep(30);" > /dev/null 2>&1 &
SELECT pg_sleep(1);
SELECT signal, successful, state, query, reported, pg_sleep(1)
FROM pgl_ddl_deploy.kill_blockers('terminate','public','foo');
/*** With <=1.5, it showed this. But it should kill the process.
signal | successful | state | query | reported | pg_sleep
--------+------------+-------+-------+----------+----------
(0 rows)
***/

/*** TEST REFERENCED BY TABLE BLOCKER KILLER ***/
-- Same workflow as above, but instead select from a table which has a pkey (foo) which is referenced by another table being altered (foo3)
\! PGOPTIONS='--client-min-messages=warning' psql -d contrib_regression -c "BEGIN; SELECT * FROM public.foo; SELECT pg_sleep(30);" > /dev/null 2>&1 &
SELECT pg_sleep(1);
SELECT signal, successful, state, query, reported, pg_sleep(1)
FROM pgl_ddl_deploy.kill_blockers('terminate','public','foo3');
/*** With <=1.5, it showed this. But it should kill the process.
signal | successful | state | query | reported | pg_sleep
--------+------------+-------+-------+----------+----------
(0 rows)
***/

SET lock_timeout TO 1000;
DROP TABLE public.foo CASCADE;
-- With <=1.5, lock is still in place leading to ERROR: canceling statement due to lock timeout
DROP TABLE public.foo3 CASCADE;
-- With <=1.5, lock is still in place leading to ERROR: canceling statement due to lock timeout
TABLE bar;
DROP TABLE public.bar CASCADE;

Expand Down

0 comments on commit bd7c393

Please sign in to comment.