Hi, On 04/02/14 12:38, Fujii Masao wrote: > ISTM that the phrase "Request queue" is not used much around the lock. > Using the phrase "wait queue" or Simon's suggestion sound better to at least > me. > Thought?
Sounds reasonable to me. Attached patch changes messages to the following: Process holding the lock: A. Wait queue: B. Processes holding the lock: A, B. Wait queue: C. Best regards, -- Christian Kruse http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml index 881b0c3..aa20807 100644 --- a/doc/src/sgml/sources.sgml +++ b/doc/src/sgml/sources.sgml @@ -251,6 +251,15 @@ ereport(ERROR, </listitem> <listitem> <para> + <function>errdetail_log_plural(const char *fmt_singuar, const char + *fmt_plural, unsigned long n, ...)</function> is like + <function>errdetail_log</>, but with support for various plural forms of + the message. + For more information see <xref linkend="nls-guidelines">. + </para> + </listitem> + <listitem> + <para> <function>errhint(const char *msg, ...)</function> supplies an optional <quote>hint</> message; this is to be used when offering suggestions about how to fix the problem, as opposed to factual details about diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index fb449a8..9620f6e 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -1209,13 +1209,23 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable) */ if (log_lock_waits && deadlock_state != DS_NOT_YET_CHECKED) { - StringInfoData buf; + StringInfoData buf, + lock_waiters_sbuf, + lock_holders_sbuf; const char *modename; long secs; int usecs; long msecs; + SHM_QUEUE *procLocks; + PROCLOCK *proclock; + bool first_holder = true, + first_waiter = true; + int lockHoldersNum = 0; initStringInfo(&buf); + initStringInfo(&lock_waiters_sbuf); + initStringInfo(&lock_holders_sbuf); + DescribeLockTag(&buf, &locallock->tag.lock); modename = GetLockmodeName(locallock->tag.lock.locktag_lockmethodid, lockmode); @@ -1225,10 +1235,67 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable) msecs = secs * 1000 + usecs / 1000; usecs = usecs % 1000; + /* + * we loop over the lock's procLocks to gather a list of all + * holders and waiters. Thus we will be able to provide more + * detailed information for lock debugging purposes. + * + * lock->procLocks contains all processes which hold or wait for + * this lock. + */ + + LWLockAcquire(partitionLock, LW_SHARED); + + procLocks = &(lock->procLocks); + proclock = (PROCLOCK *) SHMQueueNext(procLocks, procLocks, + offsetof(PROCLOCK, lockLink)); + + while (proclock) + { + /* + * we are a waiter if myProc->waitProcLock == proclock; we are + * a holder if it is NULL or something different + */ + if (proclock->tag.myProc->waitProcLock == proclock) + { + if (first_waiter) + { + appendStringInfo(&lock_waiters_sbuf, "%d", + proclock->tag.myProc->pid); + first_waiter = false; + } + else + appendStringInfo(&lock_waiters_sbuf, ", %d", + proclock->tag.myProc->pid); + } + else + { + if (first_holder) + { + appendStringInfo(&lock_holders_sbuf, "%d", + proclock->tag.myProc->pid); + first_holder = false; + } + else + appendStringInfo(&lock_holders_sbuf, ", %d", + proclock->tag.myProc->pid); + + lockHoldersNum++; + } + + proclock = (PROCLOCK *) SHMQueueNext(procLocks, &proclock->lockLink, + offsetof(PROCLOCK, lockLink)); + } + + LWLockRelease(partitionLock); + if (deadlock_state == DS_SOFT_DEADLOCK) ereport(LOG, (errmsg("process %d avoided deadlock for %s on %s by rearranging queue order after %ld.%03d ms", - MyProcPid, modename, buf.data, msecs, usecs))); + MyProcPid, modename, buf.data, msecs, usecs), + (errdetail_log_plural("Process holding the lock: %s. Wait queue: %s.", + "Processes holding the lock: %s. Wait queue: %s.", + lockHoldersNum, lock_holders_sbuf.data, lock_waiters_sbuf.data)))); else if (deadlock_state == DS_HARD_DEADLOCK) { /* @@ -1240,13 +1307,19 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable) */ ereport(LOG, (errmsg("process %d detected deadlock while waiting for %s on %s after %ld.%03d ms", - MyProcPid, modename, buf.data, msecs, usecs))); + MyProcPid, modename, buf.data, msecs, usecs), + (errdetail_log_plural("Process holding the lock: %s. Wait queue: %s.", + "Processes holding the lock: %s. Wait queue: %s.", + lockHoldersNum, lock_holders_sbuf.data, lock_waiters_sbuf.data)))); } if (myWaitStatus == STATUS_WAITING) ereport(LOG, (errmsg("process %d still waiting for %s on %s after %ld.%03d ms", - MyProcPid, modename, buf.data, msecs, usecs))); + MyProcPid, modename, buf.data, msecs, usecs), + (errdetail_log_plural("Process holding the lock: %s. Wait queue: %s.", + "Processes holding the lock: %s. Wait queue: %s.", + lockHoldersNum, lock_holders_sbuf.data, lock_waiters_sbuf.data)))); else if (myWaitStatus == STATUS_OK) ereport(LOG, (errmsg("process %d acquired %s on %s after %ld.%03d ms", @@ -1266,7 +1339,10 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable) if (deadlock_state != DS_HARD_DEADLOCK) ereport(LOG, (errmsg("process %d failed to acquire %s on %s after %ld.%03d ms", - MyProcPid, modename, buf.data, msecs, usecs))); + MyProcPid, modename, buf.data, msecs, usecs), + (errdetail_log_plural("Process holding the lock: %s. Wait queue: %s.", + "Processes holding the lock: %s. Wait queue: %s.", + lockHoldersNum, lock_holders_sbuf.data, lock_waiters_sbuf.data)))); } /* @@ -1276,6 +1352,8 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable) deadlock_state = DS_NO_DEADLOCK; pfree(buf.data); + pfree(lock_holders_sbuf.data); + pfree(lock_waiters_sbuf.data); } } while (myWaitStatus == STATUS_WAITING); diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index 8705586..d28b749 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -937,6 +937,28 @@ errdetail_log(const char *fmt,...) return 0; /* return value does not matter */ } +/* + * errdetail_log_plural --- add a detail_log error message text to the current error + * with support for pluralization of the message text + */ +int +errdetail_log_plural(const char *fmt_singular, const char *fmt_plural, + unsigned long n,...) +{ + ErrorData *edata = &errordata[errordata_stack_depth]; + MemoryContext oldcontext; + + recursion_depth++; + CHECK_STACK_DEPTH(); + oldcontext = MemoryContextSwitchTo(edata->assoc_context); + + EVALUATE_MESSAGE_PLURAL(edata->domain, detail_log, false); + + MemoryContextSwitchTo(oldcontext); + recursion_depth--; + return 0; /* return value does not matter */ +} + /* * errdetail_plural --- add a detail error message text to the current error, diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h index d7916c2..427d52d 100644 --- a/src/include/utils/elog.h +++ b/src/include/utils/elog.h @@ -182,6 +182,14 @@ errdetail_log(const char *fmt,...) __attribute__((format(PG_PRINTF_ATTRIBUTE, 1, 2))); extern int +errdetail_log_plural(const char *fmt_singular, const char *fmt_plural, + unsigned long n,...) +/* This extension allows gcc to check the format string for consistency with + the supplied arguments. */ +__attribute__((format(PG_PRINTF_ATTRIBUTE, 1, 4))) +__attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 4))); + +extern int errdetail_plural(const char *fmt_singular, const char *fmt_plural, unsigned long n,...) /* This extension allows gcc to check the format string for consistency with
pgpFfsNLiyfWT.pgp
Description: PGP signature