From 83cc9590ce3d345c3a487dde9ccc9b7d6d48999e Mon Sep 17 00:00:00 2001 From: Asim R P Date: Tue, 22 Oct 2019 12:21:30 +0530 Subject: [PATCH] Properly determine the waiting backends in an isolation2 test The method used in reader_waits_for_lock test to identify a reader backend that is waiting and belongs to the desired session was wrong. It used a separate distributed table to record the desired session ID, and later queried that table in utility mode. However, the utility mode connection was established with the segment that didn't receive the session ID tuple. Fix it by identifying the reader that is waiting on a lock from pg_locks and the session it belongs to based on query string in pg_stat_activity, as suggested by Heikki. Fixes GitHub issue #8830. Reviewed thoroughly and patiently by Heikki Linnakangas. --- .../expected/reader_waits_for_lock.out | 41 ++++++++++------- .../isolation2/sql/reader_waits_for_lock.sql | 46 ++++++++++++------- 2 files changed, 55 insertions(+), 32 deletions(-) diff --git a/src/test/isolation2/expected/reader_waits_for_lock.out b/src/test/isolation2/expected/reader_waits_for_lock.out index 03e6e9eaa1..8f3c9c552f 100644 --- a/src/test/isolation2/expected/reader_waits_for_lock.out +++ b/src/test/isolation2/expected/reader_waits_for_lock.out @@ -1,19 +1,16 @@ --- This test validates, reader and writer both wait if lock is not --- available. There used to be a bug where reader didn't wait even if lock was --- held by some other session. - --- Helper function -CREATE or REPLACE FUNCTION check_session_processes_are_blocked (session_number int) /*in func*/ RETURNS bool AS $$ declare retries int; /* in func */ begin /* in func */ retries := 1200; /* in func */ loop /* in func */ if (SELECT count(case when waiting then 1 end) = count(*) all_waiting FROM pg_stat_activity where sess_id = session_number) then /* in func */ return true; /* in func */ end if; /* in func */ if retries <= 0 then /* in func */ return false; /* in func */ end if; /* in func */ perform pg_sleep(0.1); /* in func */ retries := retries - 1; /* in func */ end loop; /* in func */ end; /* in func */ $$ language plpgsql; -CREATE +-- This test validates that a reader process waits if lock is not +-- available. There used to be a bug where reader didn't wait even if +-- lock was held by some other session. -- setup +CREATE or REPLACE FUNCTION check_readers_are_blocked () RETURNS bool AS $$ declare retries int; /* in func */ begin retries := 1200; /* in func */ loop if (SELECT count(*) > 0 as reader_waits from pg_locks l join pg_stat_activity a on a.pid = l.pid and a.query like '%reader_waits_for_lock_table%' and not a.pid = pg_backend_pid() and l.granted = false and l.mppiswriter = false) then return true; /* in func */ end if; /* in func */ if retries <= 0 then return false; /* in func */ end if; /* in func */ perform pg_sleep(0.1); /* in func */ retries := retries - 1; /* in func */ end loop; /* in func */ end; /* in func */ $$ language plpgsql; +CREATE 1: create table reader_waits_for_lock_table(a int, b int) distributed by (a); CREATE 1: insert into reader_waits_for_lock_table select 1, 1; INSERT 1 --- save session id -1: CREATE TABLE reader_waits_for_lock_table_sessionid(a, setting) AS SELECT 1, setting::int FROM pg_settings WHERE name = 'gp_session_id' distributed by (a); -CREATE 1 + +-- Aquire a conflicting lock in utility mode on seg0. 0U: BEGIN; BEGIN 0U: LOCK reader_waits_for_lock_table IN ACCESS EXCLUSIVE MODE; @@ -26,13 +23,25 @@ LOCK --------------- -1 (1 row) --- creates reader and writer gang +-- Run the same query involving at least one reader gang. It should +-- block this time. 1&: SELECT t1.* FROM reader_waits_for_lock_table t1 INNER JOIN reader_waits_for_lock_table t2 ON t1.b = t2.b; --- all processes in the session 1 should be blocked -0U: select check_session_processes_are_blocked((SELECT setting FROM reader_waits_for_lock_table_sessionid)); - check_session_processes_are_blocked -------------------------------------- - t +-- At least one reader process from session 1 should be blocked on +-- AccessExclusiveLock held by 0U. That distinct is needed because +-- plans for above select query differ between Orca and planner. +-- Planner generates three slices such that the two reader backends +-- are blocked for the lock. Orca generates two slices such that the +-- reader and as well as the writer are blocked. We get two rows (due +-- to "mppwriter=false" predicate) with planner and one row with Orca. +0U: select check_readers_are_blocked(); + check_readers_are_blocked +--------------------------- + t +(1 row) +0U: select distinct relation::regclass, mode, query from pg_locks l join pg_stat_activity a on a.pid = l.pid and a.query like '%reader_waits_for_lock_table%' and not a.pid = pg_backend_pid() and l.granted = false and l.mppiswriter = false; + relation | mode | query +-----------------------------+-----------------+----------------------------------------------------------------------------------------------------------- + reader_waits_for_lock_table | AccessShareLock | SELECT t1.* FROM reader_waits_for_lock_table t1 INNER JOIN reader_waits_for_lock_table t2 ON t1.b = t2.b; (1 row) 0U: COMMIT; COMMIT diff --git a/src/test/isolation2/sql/reader_waits_for_lock.sql b/src/test/isolation2/sql/reader_waits_for_lock.sql index 4b3139c8f4..b200c63c96 100644 --- a/src/test/isolation2/sql/reader_waits_for_lock.sql +++ b/src/test/isolation2/sql/reader_waits_for_lock.sql @@ -1,20 +1,24 @@ --- This test validates, reader and writer both wait if lock is not --- available. There used to be a bug where reader didn't wait even if lock was --- held by some other session. +-- This test validates that a reader process waits if lock is not +-- available. There used to be a bug where reader didn't wait even if +-- lock was held by some other session. --- Helper function -CREATE or REPLACE FUNCTION check_session_processes_are_blocked (session_number int) /*in func*/ +-- setup +CREATE or REPLACE FUNCTION check_readers_are_blocked () RETURNS bool AS $$ declare retries int; /* in func */ -begin /* in func */ +begin retries := 1200; /* in func */ - loop /* in func */ - if (SELECT count(case when waiting then 1 end) = count(*) all_waiting FROM pg_stat_activity where sess_id = session_number) then /* in func */ + loop + if (SELECT count(*) > 0 as reader_waits from + pg_locks l join pg_stat_activity a on a.pid = l.pid + and a.query like '%reader_waits_for_lock_table%' + and not a.pid = pg_backend_pid() + and l.granted = false and l.mppiswriter = false) then return true; /* in func */ end if; /* in func */ - if retries <= 0 then /* in func */ + if retries <= 0 then return false; /* in func */ end if; /* in func */ perform pg_sleep(0.1); /* in func */ @@ -22,21 +26,31 @@ begin /* in func */ end loop; /* in func */ end; /* in func */ $$ language plpgsql; - --- setup 1: create table reader_waits_for_lock_table(a int, b int) distributed by (a); 1: insert into reader_waits_for_lock_table select 1, 1; --- save session id -1: CREATE TABLE reader_waits_for_lock_table_sessionid(a, setting) AS SELECT 1, setting::int FROM pg_settings WHERE name = 'gp_session_id' distributed by (a); + +-- Aquire a conflicting lock in utility mode on seg0. 0U: BEGIN; 0U: LOCK reader_waits_for_lock_table IN ACCESS EXCLUSIVE MODE; -- A utility mode connection should not have valid gp_session_id, else -- locks aquired by it may not confict with locks requested by a -- normal mode backend. 0U: show gp_session_id; --- creates reader and writer gang +-- Run the same query involving at least one reader gang. It should +-- block this time. 1&: SELECT t1.* FROM reader_waits_for_lock_table t1 INNER JOIN reader_waits_for_lock_table t2 ON t1.b = t2.b; --- all processes in the session 1 should be blocked -0U: select check_session_processes_are_blocked((SELECT setting FROM reader_waits_for_lock_table_sessionid)); +-- At least one reader process from session 1 should be blocked on +-- AccessExclusiveLock held by 0U. That distinct is needed because +-- plans for above select query differ between Orca and planner. +-- Planner generates three slices such that the two reader backends +-- are blocked for the lock. Orca generates two slices such that the +-- reader and as well as the writer are blocked. We get two rows (due +-- to "mppwriter=false" predicate) with planner and one row with Orca. +0U: select check_readers_are_blocked(); +0U: select distinct relation::regclass, mode, query + from pg_locks l join pg_stat_activity a on a.pid = l.pid + and a.query like '%reader_waits_for_lock_table%' + and not a.pid = pg_backend_pid() + and l.granted = false and l.mppiswriter = false; 0U: COMMIT; 1<: -- GitLab