From 890d2182a2c425aaa80f9bf9f7116d31e0c6538e Mon Sep 17 00:00:00 2001 From: Fujii Masao Date: Tue, 9 Feb 2021 18:30:40 +0900 Subject: [PATCH] Revert "Display the time when the process started waiting for the lock, in pg_locks." This reverts commit 3b733fcd04195399db56f73f0616b4f5c6828e18. Per buildfarm members prion and rorqual. --- contrib/amcheck/expected/check_btree.out | 4 ++-- doc/src/sgml/catalogs.sgml | 13 ------------- src/backend/storage/ipc/standby.c | 24 +----------------------- src/backend/storage/lmgr/lock.c | 8 -------- src/backend/storage/lmgr/proc.c | 19 ------------------- src/backend/utils/adt/lockfuncs.c | 9 +-------- src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_proc.dat | 6 +++--- src/include/storage/lock.h | 3 --- src/include/storage/proc.h | 2 -- src/test/regress/expected/rules.out | 5 ++--- 11 files changed, 10 insertions(+), 85 deletions(-) diff --git a/contrib/amcheck/expected/check_btree.out b/contrib/amcheck/expected/check_btree.out index 5a3f1ef737..13848b7449 100644 --- a/contrib/amcheck/expected/check_btree.out +++ b/contrib/amcheck/expected/check_btree.out @@ -97,8 +97,8 @@ SELECT bt_index_parent_check('bttest_b_idx'); SELECT * FROM pg_locks WHERE relation = ANY(ARRAY['bttest_a', 'bttest_a_idx', 'bttest_b', 'bttest_b_idx']::regclass[]) AND pid = pg_backend_pid(); - locktype | database | relation | page | tuple | virtualxid | transactionid | classid | objid | objsubid | virtualtransaction | pid | mode | granted | fastpath | waitstart -----------+----------+----------+------+-------+------------+---------------+---------+-------+----------+--------------------+-----+------+---------+----------+----------- + locktype | database | relation | page | tuple | virtualxid | transactionid | classid | objid | objsubid | virtualtransaction | pid | mode | granted | fastpath +----------+----------+----------+------+-------+------------+---------------+---------+-------+----------+--------------------+-----+------+---------+---------- (0 rows) COMMIT; diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 9085d2e64c..ea222c0464 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -10604,19 +10604,6 @@ SCRAM-SHA-256$<iteration count>:&l lock table - - - - waitstart timestamptz - - - Time when the server process started waiting for this lock, - or null if the lock is held. - Note that this can be null for a very short period of time after - the wait started even though granted - is false. - - diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c index 5877a60715..39a30c00f7 100644 --- a/src/backend/storage/ipc/standby.c +++ b/src/backend/storage/ipc/standby.c @@ -540,34 +540,12 @@ void ResolveRecoveryConflictWithLock(LOCKTAG locktag, bool logging_conflict) { TimestampTz ltime; - TimestampTz now; Assert(InHotStandby); ltime = GetStandbyLimitTime(); - now = GetCurrentTimestamp(); - /* - * Update waitStart if first time through after the startup process - * started waiting for the lock. It should not be updated every time - * ResolveRecoveryConflictWithLock() is called during the wait. - * - * Use the current time obtained for comparison with ltime as waitStart - * (i.e., the time when this process started waiting for the lock). Since - * getting the current time newly can cause overhead, we reuse the - * already-obtained time to avoid that overhead. - * - * Note that waitStart is updated without holding the lock table's - * partition lock, to avoid the overhead by additional lock acquisition. - * This can cause "waitstart" in pg_locks to become NULL for a very short - * period of time after the wait started even though "granted" is false. - * This is OK in practice because we can assume that users are likely to - * look at "waitstart" when waiting for the lock for a long time. - */ - if (pg_atomic_read_u64(&MyProc->waitStart) == 0) - pg_atomic_write_u64(&MyProc->waitStart, now); - - if (now >= ltime && ltime != 0) + if (GetCurrentTimestamp() >= ltime && ltime != 0) { /* * We're already behind, so clear a path as quickly as possible. diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c index 108b4d9023..79c1cf9b8b 100644 --- a/src/backend/storage/lmgr/lock.c +++ b/src/backend/storage/lmgr/lock.c @@ -3619,12 +3619,6 @@ GetLockStatusData(void) instance->leaderPid = proc->pid; instance->fastpath = true; - /* - * Successfully taking fast path lock means there were no - * conflicting locks. - */ - instance->waitStart = 0; - el++; } @@ -3652,7 +3646,6 @@ GetLockStatusData(void) instance->pid = proc->pid; instance->leaderPid = proc->pid; instance->fastpath = true; - instance->waitStart = 0; el++; } @@ -3705,7 +3698,6 @@ GetLockStatusData(void) instance->pid = proc->pid; instance->leaderPid = proclock->groupLeader->pid; instance->fastpath = false; - instance->waitStart = (TimestampTz) pg_atomic_read_u64(&proc->waitStart); el++; } diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index f059d60a0f..c87ffc6549 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -402,7 +402,6 @@ InitProcess(void) MyProc->lwWaitMode = 0; MyProc->waitLock = NULL; MyProc->waitProcLock = NULL; - pg_atomic_init_u64(&MyProc->waitStart, 0); #ifdef USE_ASSERT_CHECKING { int i; @@ -1263,23 +1262,6 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable) } else enable_timeout_after(DEADLOCK_TIMEOUT, DeadlockTimeout); - - /* - * Use the current time obtained for the deadlock timeout timer as - * waitStart (i.e., the time when this process started waiting for the - * lock). Since getting the current time newly can cause overhead, we - * reuse the already-obtained time to avoid that overhead. - * - * Note that waitStart is updated without holding the lock table's - * partition lock, to avoid the overhead by additional lock - * acquisition. This can cause "waitstart" in pg_locks to become NULL - * for a very short period of time after the wait started even though - * "granted" is false. This is OK in practice because we can assume - * that users are likely to look at "waitstart" when waiting for the - * lock for a long time. - */ - pg_atomic_write_u64(&MyProc->waitStart, - get_timeout_start_time(DEADLOCK_TIMEOUT)); } else if (log_recovery_conflict_waits) { @@ -1696,7 +1678,6 @@ ProcWakeup(PGPROC *proc, ProcWaitStatus waitStatus) proc->waitLock = NULL; proc->waitProcLock = NULL; proc->waitStatus = waitStatus; - pg_atomic_write_u64(&MyProc->waitStart, 0); /* And awaken it */ SetLatch(&proc->procLatch); diff --git a/src/backend/utils/adt/lockfuncs.c b/src/backend/utils/adt/lockfuncs.c index 97f0265c12..b1cf5b79a7 100644 --- a/src/backend/utils/adt/lockfuncs.c +++ b/src/backend/utils/adt/lockfuncs.c @@ -63,7 +63,7 @@ typedef struct } PG_Lock_Status; /* Number of columns in pg_locks output */ -#define NUM_LOCK_STATUS_COLUMNS 16 +#define NUM_LOCK_STATUS_COLUMNS 15 /* * VXIDGetDatum - Construct a text representation of a VXID @@ -142,8 +142,6 @@ pg_lock_status(PG_FUNCTION_ARGS) BOOLOID, -1, 0); TupleDescInitEntry(tupdesc, (AttrNumber) 15, "fastpath", BOOLOID, -1, 0); - TupleDescInitEntry(tupdesc, (AttrNumber) 16, "waitstart", - TIMESTAMPTZOID, -1, 0); funcctx->tuple_desc = BlessTupleDesc(tupdesc); @@ -338,10 +336,6 @@ pg_lock_status(PG_FUNCTION_ARGS) values[12] = CStringGetTextDatum(GetLockmodeName(instance->locktag.locktag_lockmethodid, mode)); values[13] = BoolGetDatum(granted); values[14] = BoolGetDatum(instance->fastpath); - if (!granted && instance->waitStart != 0) - values[15] = TimestampTzGetDatum(instance->waitStart); - else - nulls[15] = true; tuple = heap_form_tuple(funcctx->tuple_desc, values, nulls); result = HeapTupleGetDatum(tuple); @@ -412,7 +406,6 @@ pg_lock_status(PG_FUNCTION_ARGS) values[12] = CStringGetTextDatum("SIReadLock"); values[13] = BoolGetDatum(true); values[14] = BoolGetDatum(false); - nulls[15] = true; tuple = heap_form_tuple(funcctx->tuple_desc, values, nulls); result = HeapTupleGetDatum(tuple); diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index c1a5dd4ee7..638830aaac 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -53,6 +53,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 202102091 +#define CATALOG_VERSION_NO 202102021 #endif diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 1487710d59..4e0c9be58c 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -5994,9 +5994,9 @@ { oid => '1371', descr => 'view system lock information', proname => 'pg_lock_status', prorows => '1000', proretset => 't', provolatile => 'v', prorettype => 'record', proargtypes => '', - proallargtypes => '{text,oid,oid,int4,int2,text,xid,oid,oid,int2,text,int4,text,bool,bool,timestamptz}', - proargmodes => '{o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}', - proargnames => '{locktype,database,relation,page,tuple,virtualxid,transactionid,classid,objid,objsubid,virtualtransaction,pid,mode,granted,fastpath,waitstart}', + proallargtypes => '{text,oid,oid,int4,int2,text,xid,oid,oid,int2,text,int4,text,bool,bool}', + proargmodes => '{o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}', + proargnames => '{locktype,database,relation,page,tuple,virtualxid,transactionid,classid,objid,objsubid,virtualtransaction,pid,mode,granted,fastpath}', prosrc => 'pg_lock_status' }, { oid => '2561', descr => 'get array of PIDs of sessions blocking specified backend PID from acquiring a heavyweight lock', diff --git a/src/include/storage/lock.h b/src/include/storage/lock.h index 9b2a421c32..68a3487d49 100644 --- a/src/include/storage/lock.h +++ b/src/include/storage/lock.h @@ -22,7 +22,6 @@ #include "storage/lockdefs.h" #include "storage/lwlock.h" #include "storage/shmem.h" -#include "utils/timestamp.h" /* struct PGPROC is declared in proc.h, but must forward-reference it */ typedef struct PGPROC PGPROC; @@ -447,8 +446,6 @@ typedef struct LockInstanceData LOCKMODE waitLockMode; /* lock awaited by this PGPROC, if any */ BackendId backend; /* backend ID of this PGPROC */ LocalTransactionId lxid; /* local transaction ID of this PGPROC */ - TimestampTz waitStart; /* time at which this PGPROC started waiting - * for lock */ int pid; /* pid of this PGPROC */ int leaderPid; /* pid of group leader; = pid if no group */ bool fastpath; /* taken via fastpath? */ diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index a777cb64a1..683ab64f76 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -181,8 +181,6 @@ struct PGPROC LOCKMODE waitLockMode; /* type of lock we're waiting for */ LOCKMASK heldLocks; /* bitmask for lock types already held on this * lock object by this backend */ - pg_atomic_uint64 waitStart; /* time at which wait for lock acquisition - * started */ bool delayChkpt; /* true if this proc delays checkpoint start */ diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 10a1f34ebc..b632d9f2ea 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1394,9 +1394,8 @@ pg_locks| SELECT l.locktype, l.pid, l.mode, l.granted, - l.fastpath, - l.waitstart - FROM pg_lock_status() l(locktype, database, relation, page, tuple, virtualxid, transactionid, classid, objid, objsubid, virtualtransaction, pid, mode, granted, fastpath, waitstart); + l.fastpath + FROM pg_lock_status() l(locktype, database, relation, page, tuple, virtualxid, transactionid, classid, objid, objsubid, virtualtransaction, pid, mode, granted, fastpath); pg_matviews| SELECT n.nspname AS schemaname, c.relname AS matviewname, pg_get_userbyid(c.relowner) AS matviewowner,