Hi, On 03/02/14 17:59, Fujii Masao wrote: > Since you added errdetail_log_plural(), ISTM that you need to update > sources.sgml.
[x] Fixed. > >> While I'm griping, this message isn't even trying to follow the project's > >> message style guidelines. Detail or context messages are supposed to be > >> complete sentence(s), with capitalization and punctuation to match. > > > > Hm, I hope I fixed it in this version of the patch. > > Current message doesn't look like complete sentence yet... We would > need to use something like "Processes X, Y are holding while Z is waiting > for the lock.". I could not come up with good message, though.. The problem is that we have two potential plural cases in this message. That leads to the need to formulate the second part independently from singular/plural. I tried to improve a little bit and propose this message: Singular: "The following process is holding the lock: A. The request queue consists of: B." Plural: "Following processes are holding the lock: A, B. The request queue consists of: C." Attached you will find an updated patch. 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..da72c82 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("The following process is holding the lock: %s. The request queue consists of: %s.",
+ "Following processes are holding the lock: %s. The request queue consists of: %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("The following process is holding the lock: %s. The request queue consists of: %s.",
+ "Following processes are holding the lock: %s. The request queue consists of: %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("The following process is holding the lock: %s. The request queue consists of: %s.",
+ "Following processes are holding the lock: %s. The request queue consists of: %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("The following process is holding the lock: %s. The request queue consists of: %s.",
+ "Following processes are holding the lock: %s. The request queue consists of: %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
pgpwNTBf5CY1M.pgp
Description: PGP signature
