Here's an adjusted version.  In this one, the extra info is not used to
construct a string from pieces, but instead it puts it at the end, like
this:

LOG:  process 18899 still waiting for ShareLock on transaction 697 after 
1000.203 ms
CONTEXT:  while operating on tuple (0,2) in relation "public"."foo" of database 
"postgres": updating tuple

This way, each part can sensibly be translated.  In fact I did translate
one instance to test it at work, and it looks good to me:

LOG:  el proceso 22555 adquirió ShareLock en transacción 705 después de 
1514.017 ms
CONTEXT:  mientras se operaba en la tupla (0,2) en la relación "public"."foo" 
de la base de datos «postgres»: actualizando tupla

Now there might be bikeshedding on the exact wording I've chosen for
each instance of context setup, but I expect it's a fairly minor point
now.

One remaining issue is that now ConditionalXactLockTableWait doesn't set
up error context info.  We could solve this by having a common routine
that serves both that one and XactLockTableWait (much like
Do_MultiXactIdWait does), but I'm not sure it's worth the trouble.
Thoughts?

Another thing that jumped at me is that passing a TID but not a Relation
is fairly useless as it stands.  We might try to add some more stuff
later, such as printing tuple contents as previous versions of the patch
did, but given the opposition the idea had previously, I'm not sure
that's ever going to happen.  If we decide that TID-but-not-Relation is
not a useful case, then we can trim the number of messages to translate.

Opinions on these points please?  I intend to push this patch tomorrow.


Note: the changes to backend/po/es.po are illustrative only.  Those are
not going in with the patch.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index e2337ac..14837f9 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -105,11 +105,12 @@ static void GetMultiXactIdHintBits(MultiXactId multi, uint16 *new_infomask,
 					   uint16 *new_infomask2);
 static TransactionId MultiXactIdGetUpdateXid(TransactionId xmax,
 						uint16 t_infomask);
-static void MultiXactIdWait(MultiXactId multi, MultiXactStatus status,
-				int *remaining, uint16 infomask);
-static bool ConditionalMultiXactIdWait(MultiXactId multi,
-						   MultiXactStatus status, int *remaining,
-						   uint16 infomask);
+static void MultiXactIdWait(MultiXactId multi, MultiXactStatus status, uint16 infomask,
+				Relation rel, ItemPointer ctid, const char *opr,
+				int *remaining);
+static bool ConditionalMultiXactIdWait(MultiXactId multi, MultiXactStatus status,
+						   uint16 infomask, Relation rel, ItemPointer ctid,
+						   const char *opr, int *remaining);
 static XLogRecPtr log_heap_new_cid(Relation relation, HeapTuple tup);
 static HeapTuple ExtractReplicaIdentity(Relation rel, HeapTuple tup, bool key_modified,
 										bool *copy);
@@ -2714,8 +2715,9 @@ l1:
 		if (infomask & HEAP_XMAX_IS_MULTI)
 		{
 			/* wait for multixact */
-			MultiXactIdWait((MultiXactId) xwait, MultiXactStatusUpdate,
-							NULL, infomask);
+			MultiXactIdWait((MultiXactId) xwait, MultiXactStatusUpdate, infomask,
+							relation, &tp.t_data->t_ctid, "deleting tuple",
+							NULL);
 			LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
 
 			/*
@@ -2741,7 +2743,8 @@ l1:
 		else
 		{
 			/* wait for regular transaction to end */
-			XactLockTableWait(xwait);
+			XactLockTableWait(xwait, relation, &tp.t_data->t_ctid,
+							  "deleting tuple");
 			LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
 
 			/*
@@ -3266,8 +3269,9 @@ l2:
 			int			remain;
 
 			/* wait for multixact */
-			MultiXactIdWait((MultiXactId) xwait, mxact_status, &remain,
-							infomask);
+			MultiXactIdWait((MultiXactId) xwait, mxact_status, infomask,
+							relation, &oldtup.t_data->t_ctid, "updating tuple",
+							&remain);
 			LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
 
 			/*
@@ -3341,7 +3345,8 @@ l2:
 			else
 			{
 				/* wait for regular transaction to end */
-				XactLockTableWait(xwait);
+				XactLockTableWait(xwait, relation, &oldtup.t_data->t_ctid,
+								  "updating tuple");
 				LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
 
 				/*
@@ -4402,14 +4407,18 @@ l3:
 				if (nowait)
 				{
 					if (!ConditionalMultiXactIdWait((MultiXactId) xwait,
-													status, NULL, infomask))
+													status, infomask, relation,
+													&tuple->t_data->t_ctid,
+													"locking tuple", NULL))
 						ereport(ERROR,
 								(errcode(ERRCODE_LOCK_NOT_AVAILABLE),
 								 errmsg("could not obtain lock on row in relation \"%s\"",
 										RelationGetRelationName(relation))));
 				}
 				else
-					MultiXactIdWait((MultiXactId) xwait, status, NULL, infomask);
+					MultiXactIdWait((MultiXactId) xwait, status, infomask,
+									relation, &tuple->t_data->t_ctid,
+									"locking tuple", NULL);
 
 				/* if there are updates, follow the update chain */
 				if (follow_updates &&
@@ -4464,7 +4473,8 @@ l3:
 										RelationGetRelationName(relation))));
 				}
 				else
-					XactLockTableWait(xwait);
+					XactLockTableWait(xwait, relation, &tuple->t_data->t_ctid,
+									  "locking tuple");
 
 				/* if there are updates, follow the update chain */
 				if (follow_updates &&
@@ -5151,7 +5161,9 @@ l4:
 					if (needwait)
 					{
 						LockBuffer(buf, BUFFER_LOCK_UNLOCK);
-						XactLockTableWait(members[i].xid);
+						XactLockTableWait(members[i].xid, rel,
+										  &mytup.t_data->t_ctid,
+										  "locking updated version of tuple");
 						pfree(members);
 						goto l4;
 					}
@@ -5211,7 +5223,8 @@ l4:
 				if (needwait)
 				{
 					LockBuffer(buf, BUFFER_LOCK_UNLOCK);
-					XactLockTableWait(rawxmax);
+					XactLockTableWait(rawxmax, rel, &mytup.t_data->t_ctid,
+									  "locking updated version of tuple");
 					goto l4;
 				}
 				if (res != HeapTupleMayBeUpdated)
@@ -6076,6 +6089,15 @@ HeapTupleGetUpdateXid(HeapTupleHeader tuple)
  * Do_MultiXactIdWait
  *		Actual implementation for the two functions below.
  *
+ * 'multi', 'status' and 'infomask' indicate what to sleep on (the status is
+ * needed to ensure we only sleep on conflicting members, and the infomask is
+ * used to optimize multixact access in case it's a lock-only multi); 'nowait'
+ * indicates whether to use conditional lock acquisition, to allow callers to
+ * fail if lock is unavailable.  'rel', 'ctid' and 'opr' are used to set up
+ * context information for error messages.  'remaining', if not NULL, receives
+ * the number of members that are still running, including any (non-aborted)
+ * subtransactions of our own transaction.
+ *
  * We do this by sleeping on each member using XactLockTableWait.  Any
  * members that belong to the current backend are *not* waited for, however;
  * this would not merely be useless but would lead to Assert failure inside
@@ -6093,7 +6115,9 @@ HeapTupleGetUpdateXid(HeapTupleHeader tuple)
  */
 static bool
 Do_MultiXactIdWait(MultiXactId multi, MultiXactStatus status,
-				   int *remaining, uint16 infomask, bool nowait)
+				   uint16 infomask, bool nowait,
+				   Relation rel, ItemPointer ctid, const char *opr,
+				   int *remaining)
 {
 	bool		allow_old;
 	bool		result = true;
@@ -6130,6 +6154,12 @@ Do_MultiXactIdWait(MultiXactId multi, MultiXactStatus status,
 			/*
 			 * This member conflicts with our multi, so we have to sleep (or
 			 * return failure, if asked to avoid waiting.)
+			 *
+			 * Note that we don't set up an error context callback ourselves,
+			 * but instead we pass the info down to XactLockTableWait.  This
+			 * might seem a bit wasteful because the context is set up and tore
+			 * down for each member of the multixact, but in reality it should
+			 * be barely noticeable, and it avoids duplicate code.
 			 */
 			if (nowait)
 			{
@@ -6138,7 +6168,7 @@ Do_MultiXactIdWait(MultiXactId multi, MultiXactStatus status,
 					break;
 			}
 			else
-				XactLockTableWait(memxid);
+				XactLockTableWait(memxid, rel, ctid, opr);
 		}
 
 		pfree(members);
@@ -6159,13 +6189,14 @@ Do_MultiXactIdWait(MultiXactId multi, MultiXactStatus status,
  *
  * We return (in *remaining, if not NULL) the number of members that are still
  * running, including any (non-aborted) subtransactions of our own transaction.
- *
  */
 static void
-MultiXactIdWait(MultiXactId multi, MultiXactStatus status,
-				int *remaining, uint16 infomask)
+MultiXactIdWait(MultiXactId multi, MultiXactStatus status, uint16 infomask,
+				Relation rel, ItemPointer ctid, const char *opr,
+				int *remaining)
 {
-	Do_MultiXactIdWait(multi, status, remaining, infomask, false);
+	(void) Do_MultiXactIdWait(multi, status, infomask, false,
+							  rel, ctid, opr, remaining);
 }
 
 /*
@@ -6183,9 +6214,11 @@ MultiXactIdWait(MultiXactId multi, MultiXactStatus status,
  */
 static bool
 ConditionalMultiXactIdWait(MultiXactId multi, MultiXactStatus status,
-						   int *remaining, uint16 infomask)
+						   uint16 infomask, Relation rel, ItemPointer ctid,
+						   const char *opr, int *remaining)
 {
-	return Do_MultiXactIdWait(multi, status, remaining, infomask, true);
+	return Do_MultiXactIdWait(multi, status, infomask, true,
+							  rel, ctid, opr, remaining);
 }
 
 /*
diff --git a/src/backend/access/nbtree/nbtinsert.c b/src/backend/access/nbtree/nbtinsert.c
index 5f7953f..53685d0 100644
--- a/src/backend/access/nbtree/nbtinsert.c
+++ b/src/backend/access/nbtree/nbtinsert.c
@@ -164,7 +164,7 @@ top:
 		{
 			/* Have to wait for the other guy ... */
 			_bt_relbuf(rel, buf);
-			XactLockTableWait(xwait);
+			XactLockTableWait(xwait, rel, &itup->t_tid, "inserting index tuple");
 			/* start over... */
 			_bt_freestack(stack);
 			goto top;
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 877d767..3d29859 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -2295,7 +2295,9 @@ IndexBuildHeapScan(Relation heapRelation,
 							 * Must drop the lock on the buffer before we wait
 							 */
 							LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK);
-							XactLockTableWait(xwait);
+							XactLockTableWait(xwait, heapRelation,
+											  &heapTuple->t_data->t_ctid,
+											  "creating unique index tuple");
 							goto recheck;
 						}
 					}
@@ -2341,7 +2343,9 @@ IndexBuildHeapScan(Relation heapRelation,
 							 * Must drop the lock on the buffer before we wait
 							 */
 							LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK);
-							XactLockTableWait(xwait);
+							XactLockTableWait(xwait, heapRelation,
+											  &heapTuple->t_data->t_ctid,
+											  "creating unique index tuple");
 							goto recheck;
 						}
 
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 9e3d879..5132b60 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1982,7 +1982,9 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
 			if (TransactionIdIsValid(SnapshotDirty.xmax))
 			{
 				ReleaseBuffer(buffer);
-				XactLockTableWait(SnapshotDirty.xmax);
+				XactLockTableWait(SnapshotDirty.xmax,
+								  relation, &tuple.t_data->t_ctid,
+								  "rechecking updated version of tuple");
 				continue;		/* loop back to repeat heap_fetch */
 			}
 
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index 46895b2..702e7f1 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -1307,7 +1307,8 @@ retry:
 		if (TransactionIdIsValid(xwait))
 		{
 			index_endscan(index_scan);
-			XactLockTableWait(xwait);
+			XactLockTableWait(xwait, heap, &tup->t_data->t_ctid,
+							  "rechecking exclusion constraint tuple");
 			goto retry;
 		}
 
diff --git a/src/backend/nls.mk b/src/backend/nls.mk
index d69722f..6914e9c 100644
--- a/src/backend/nls.mk
+++ b/src/backend/nls.mk
@@ -4,7 +4,7 @@ AVAIL_LANGUAGES  = de es fr it ja pl pt_BR ru zh_CN zh_TW
 GETTEXT_FILES    = + gettext-files
 GETTEXT_TRIGGERS = $(BACKEND_COMMON_GETTEXT_TRIGGERS) \
     GUC_check_errmsg GUC_check_errdetail GUC_check_errhint \
-    write_stderr yyerror parser_yyerror report_invalid_record
+    write_stderr yyerror parser_yyerror report_invalid_record \
 GETTEXT_FLAGS    = $(BACKEND_COMMON_GETTEXT_FLAGS) \
     GUC_check_errmsg:1:c-format \
     GUC_check_errdetail:1:c-format \
diff --git a/src/backend/po/es.po b/src/backend/po/es.po
index a948c8f..d708140 100644
--- a/src/backend/po/es.po
+++ b/src/backend/po/es.po
@@ -59,7 +59,7 @@
 "Project-Id-Version: PostgreSQL server 9.3\n"
 "Report-Msgid-Bugs-To: pgsql-b...@postgresql.org\n"
 "POT-Creation-Date: 2013-08-29 23:13+0000\n"
-"PO-Revision-Date: 2013-09-20 10:14-0300\n"
+"PO-Revision-Date: 2014-03-17 18:58-0300\n"
 "Last-Translator: Álvaro Herrera <alvhe...@alvh.no-ip.org>\n"
 "Language-Team: PgSQL Español <pgsql-es-ay...@postgresql.org>\n"
 "Language: es\n"
@@ -14990,6 +14990,9 @@
 msgid "updates on slices of fixed-length arrays not implemented"
 msgstr "no están implementadas las actualizaciones en segmentos de arrays de largo fija"
 
+msgid "updating tuple"
+msgstr "actualizando tupla"
+
 #, c-format
 msgid "upper bound cannot be less than lower bound"
 msgstr "el límite superior no puede ser menor que el límite inferior"
@@ -15170,6 +15173,9 @@
 msgid "weight out of range"
 msgstr "el peso está fuera de rango"
 
+msgid "while operating on tuple (%u,%u) in relation \"%s\".\"%s\" of database \"%s\": %s"
+msgstr "mientras se operaba en la tupla (%u,%u) en la relación \"%s\".\"%s\" de la base de datos «%s»: %s"
+
 #, c-format
 msgid "width argument position must be ended by \"$\""
 msgstr "la posición del argumento de anchura debe terminar con «$»"
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index e74053c..2cffeb0 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -1343,7 +1343,7 @@ SnapBuildFindSnapshot(SnapBuild *builder, XLogRecPtr lsn, xl_running_xacts *runn
 			if (TransactionIdIsCurrentTransactionId(xid))
 				elog(ERROR, "waiting for ourselves");
 
-			XactLockTableWait(xid);
+			XactLockTableWait(xid, NULL, NULL, NULL);
 		}
 
 		/* nothing could have built up so far, so don't perform cleanup */
diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c
index 4c61a6f..77c5679 100644
--- a/src/backend/storage/lmgr/lmgr.c
+++ b/src/backend/storage/lmgr/lmgr.c
@@ -19,10 +19,12 @@
 #include "access/transam.h"
 #include "access/xact.h"
 #include "catalog/catalog.h"
+#include "commands/dbcommands.h"
 #include "miscadmin.h"
 #include "storage/lmgr.h"
 #include "storage/procarray.h"
 #include "utils/inval.h"
+#include "utils/lsyscache.h"
 
 
 /*
@@ -471,7 +473,9 @@ XactLockTableDelete(TransactionId xid)
 /*
  *		XactLockTableWait
  *
- * Wait for the specified transaction to commit or abort.
+ * Wait for the specified transaction to commit or abort.  If we're given a
+ * relation, tid and operation name, those are used to set up an error context
+ * callback.
  *
  * Note that this does the right thing for subtransactions: if we wait on a
  * subtransaction, we will exit as soon as it aborts or its top parent commits.
@@ -481,9 +485,31 @@ XactLockTableDelete(TransactionId xid)
  * and if so wait for its parent.
  */
 void
-XactLockTableWait(TransactionId xid)
+XactLockTableWait(TransactionId xid, Relation rel, ItemPointer ctid,
+				  const char *opr_name)
 {
 	LOCKTAG		tag;
+	XactLockTableWaitLockInfo info;
+	ErrorContextCallback callback;
+
+	/*
+	 * If we're given an operation name, set up our verbose error context
+	 * callback.
+	 */
+	if (opr_name != NULL)
+	{
+		Assert(rel != NULL);
+		Assert(ctid != NULL);
+
+		info.rel = rel;
+		info.ctid = ctid;
+		info.opr_name = opr_name;
+
+		callback.callback = XactLockTableWaitErrorContextCallback;
+		callback.arg = &info;
+		callback.previous = error_context_stack;
+		error_context_stack = &callback;
+	}
 
 	for (;;)
 	{
@@ -500,6 +526,9 @@ XactLockTableWait(TransactionId xid)
 			break;
 		xid = SubTransGetParent(xid);
 	}
+
+	if (opr_name != NULL)
+		error_context_stack = callback.previous;
 }
 
 /*
@@ -534,6 +563,73 @@ ConditionalXactLockTableWait(TransactionId xid)
 }
 
 /*
+ *		XactLockTableWaitErrorContextCallback
+ *
+ * Error context callback for transaction lock. It reports operation info,
+ * tuple id, relation name and database name for tuple being operated on
+ * at the time of transaction lock.
+ *
+ */
+void
+XactLockTableWaitErrorContextCallback(void *arg)
+{
+	XactLockTableWaitLockInfo *info = (XactLockTableWaitLockInfo *) arg;
+
+	/*
+	 * To be able to create a translation for this error context we have to
+	 * have error message without altering information; for example the ctid
+	 * or the relation name may differ from call to call. So we have to
+	 * distinct between the several calls to provide the necessary arguments
+	 * to errcontext() for the previous assembled error string, although it is
+	 * not really a nice piece of code.
+	 */
+
+	if (ItemPointerIsValid(info->ctid))
+	{
+		if (info->rel != NULL)
+		{
+			errcontext("while operating on tuple (%u,%u) in relation \"%s\".\"%s\" of database \"%s\": %s",
+					   ItemPointerGetBlockNumber(info->ctid),
+					   ItemPointerGetOffsetNumber(info->ctid),
+					   get_namespace_name(RelationGetNamespace(info->rel)),
+					   RelationGetRelationName(info->rel),
+					   get_database_name(info->rel->rd_node.dbNode),
+					   _(info->opr_name));
+		}
+		else if (MyDatabaseId != InvalidOid)
+			errcontext("while operating on tuple (%u,%u) in database \"%s\": %s",
+					   ItemPointerGetBlockNumber(info->ctid),
+					   ItemPointerGetOffsetNumber(info->ctid),
+					   get_database_name(MyDatabaseId),
+					   _(info->opr_name));
+		else
+			errcontext("while operating on tuple (%u,%u): %s",
+					   ItemPointerGetBlockNumber(info->ctid),
+					   ItemPointerGetOffsetNumber(info->ctid),
+					   _(info->opr_name));
+	}
+	else
+	{
+		if (info->rel != NULL)
+			errcontext("while operating on tuple in relation \"%s\".\"%s\" of database \"%s\": %s",
+					   get_namespace_name(RelationGetNamespace(info->rel)),
+					   RelationGetRelationName(info->rel),
+					   get_database_name(info->rel->rd_node.dbNode),
+					   _(info->opr_name));
+		else if (MyDatabaseId != InvalidOid)
+			errcontext("while operating on tuple in database \"%s\": %s",
+					   get_database_name(MyDatabaseId),
+					   _(info->opr_name));
+
+		/*
+		 * We can't provide any useful info on the missing else tree: we have
+		 * no valid ctid, no valid database id and no relation. So simply
+		 * ignore this case and don't create any error context
+		 */
+	}
+}
+
+/*
  * WaitForLockersMultiple
  *		Wait until no transaction holds locks that conflict with the given
  *		locktags at the given lockmode.
diff --git a/src/include/storage/lmgr.h b/src/include/storage/lmgr.h
index e74ae21..95fe7ca 100644
--- a/src/include/storage/lmgr.h
+++ b/src/include/storage/lmgr.h
@@ -20,6 +20,22 @@
 #include "utils/rel.h"
 
 
+/*
+ * Struct to hold context info for transaction lock waits.
+ *
+ * rel is the relation being used when we need transaction lock.
+ * ctid is the tuple id of tuple being used by transaction on which
+ * we have to wait.
+ * opr_name is the operation which needs to wait on transaction lock.
+ */
+typedef struct XactLockTableWaitLockInfo
+{
+	Relation	rel;
+	ItemPointer	ctid;
+	const char *opr_name;
+} XactLockTableWaitLockInfo;
+
+
 extern void RelationInitLockInfo(Relation relation);
 
 /* Lock a relation */
@@ -54,9 +70,12 @@ extern void UnlockTuple(Relation relation, ItemPointer tid, LOCKMODE lockmode);
 /* Lock an XID (used to wait for a transaction to finish) */
 extern void XactLockTableInsert(TransactionId xid);
 extern void XactLockTableDelete(TransactionId xid);
-extern void XactLockTableWait(TransactionId xid);
+extern void XactLockTableWait(TransactionId xid, Relation rel,
+				  ItemPointer ctid, const char *opr_name);
 extern bool ConditionalXactLockTableWait(TransactionId xid);
 
+extern void XactLockTableWaitErrorContextCallback(void *arg);
+
 /* Lock VXIDs, specified by conflicting locktags */
 extern void WaitForLockers(LOCKTAG heaplocktag, LOCKMODE lockmode);
 extern void WaitForLockersMultiple(List *locktags, LOCKMODE lockmode);
diff --git a/src/nls-global.mk b/src/nls-global.mk
index da91c90..b90a3f3 100644
--- a/src/nls-global.mk
+++ b/src/nls-global.mk
@@ -57,7 +57,10 @@ BACKEND_COMMON_GETTEXT_TRIGGERS = \
     errmsg errmsg_plural:1,2 \
     errdetail errdetail_log errdetail_plural:1,2 \
     errhint \
-    errcontext
+    errcontext \
+    XactLockTableWait:4 \
+    MultiXactIdWait:6 \
+    ConditionalMultiXactIdWait:6
 BACKEND_COMMON_GETTEXT_FLAGS = \
     errmsg:1:c-format errmsg_plural:1:c-format errmsg_plural:2:c-format \
     errdetail:1:c-format errdetail_log:1:c-format errdetail_plural:1:c-format errdetail_plural:2:c-format \
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to