Hi Amit, I've been ill the last few days, so sorry for my late response.
> I have updated the patch to pass TID and operation information in > error context and changed some of the comments in code. > Let me know if the added operation information is useful, else > we can use better generic message in context. I don't think that this fixes the translation guideline issues brought up by Robert. This produces differing strings for the different cases as well and it also passes in altering data directly to the error string. It also may produce error messages that are really weird. You initialize the string with "while attempting to ". The remaining part of the function is covered by if()s which may lead to error messages like this: „while attempting to “ „while attempting to in relation "public"."xyz" of database "abc"“ „while attempting to in database "abc"“ Although this may not be very likely (because ItemPointerIsValid(&(info->ctid))) should in this case not return false). Attached you will find a new version of this patch; it slightly violates the translation guidelines as well: it assembles an error string (but it doesn't pass in altering data like ctid or things like that). I simply couldn't think of a nice solution without doing so, and after looking through the code there are a few cases (e.g. CheckTableNotInUse()) where this is done, too. If we insist of having complete strings in this case we would have to have 6 * 3 * 2 error strings in the code. Best regards, -- Christian Kruse 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..c0f5881 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -107,6 +107,8 @@ static TransactionId MultiXactIdGetUpdateXid(TransactionId xmax,
uint16 t_infomask);
static void MultiXactIdWait(MultiXactId multi, MultiXactStatus status,
int *remaining, uint16 infomask);
+static void MultiXactIdWaitExtended(Relation rel, ItemPointerData ctid, const char *opr_name,
+ MultiXactId multi, MultiXactStatus status, int *remaining, uint16 infomask);
static bool ConditionalMultiXactIdWait(MultiXactId multi,
MultiXactStatus status, int *remaining,
uint16 infomask);
@@ -2714,8 +2716,9 @@ l1:
if (infomask & HEAP_XMAX_IS_MULTI)
{
/* wait for multixact */
- MultiXactIdWait((MultiXactId) xwait, MultiXactStatusUpdate,
- NULL, infomask);
+ MultiXactIdWaitExtended(relation, tp.t_data->t_ctid, "delete",
+ (MultiXactId) xwait,
+ MultiXactStatusUpdate, NULL, infomask);
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
/*
@@ -2741,7 +2744,8 @@ l1:
else
{
/* wait for regular transaction to end */
- XactLockTableWait(xwait);
+ XactLockTableWaitExtended(relation, tp.t_data->t_ctid,
+ "delete", xwait);
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
/*
@@ -3266,8 +3270,8 @@ l2:
int remain;
/* wait for multixact */
- MultiXactIdWait((MultiXactId) xwait, mxact_status, &remain,
- infomask);
+ MultiXactIdWaitExtended(relation, oldtup.t_data->t_ctid, "update",
+ (MultiXactId) xwait, mxact_status, &remain, infomask);
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
/*
@@ -3306,7 +3310,7 @@ l2:
/*
* There was no UPDATE in the MultiXact; or it aborted. No
* TransactionIdIsInProgress() call needed here, since we called
- * MultiXactIdWait() above.
+ * MultiXactIdWaitExtended() above.
*/
if (!TransactionIdIsValid(update_xact) ||
TransactionIdDidAbort(update_xact))
@@ -3341,7 +3345,8 @@ l2:
else
{
/* wait for regular transaction to end */
- XactLockTableWait(xwait);
+ XactLockTableWaitExtended(relation, oldtup.t_data->t_ctid,
+ "update", xwait);
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
/*
@@ -4409,7 +4414,10 @@ l3:
RelationGetRelationName(relation))));
}
else
- MultiXactIdWait((MultiXactId) xwait, status, NULL, infomask);
+ MultiXactIdWaitExtended(relation, tuple->t_data->t_ctid,
+ "lock", (MultiXactId) xwait,
+ status, NULL,
+ infomask);
/* if there are updates, follow the update chain */
if (follow_updates &&
@@ -4464,7 +4472,8 @@ l3:
RelationGetRelationName(relation))));
}
else
- XactLockTableWait(xwait);
+ XactLockTableWaitExtended(relation, tuple->t_data->t_ctid,
+ "lock", xwait);
/* if there are updates, follow the update chain */
if (follow_updates &&
@@ -5151,7 +5160,9 @@ l4:
if (needwait)
{
LockBuffer(buf, BUFFER_LOCK_UNLOCK);
- XactLockTableWait(members[i].xid);
+ XactLockTableWaitExtended(rel, mytup.t_data->t_ctid,
+ "lock updated",
+ members[i].xid);
pfree(members);
goto l4;
}
@@ -5211,7 +5222,8 @@ l4:
if (needwait)
{
LockBuffer(buf, BUFFER_LOCK_UNLOCK);
- XactLockTableWait(rawxmax);
+ XactLockTableWaitExtended(rel, mytup.t_data->t_ctid,
+ "lock updated", rawxmax);
goto l4;
}
if (res != HeapTupleMayBeUpdated)
@@ -6169,6 +6181,36 @@ MultiXactIdWait(MultiXactId multi, MultiXactStatus status,
}
/*
+ * MultiXactIdWaitExtended
+ * Sets up the error context callback for reporting more
+ * information of locked tuple.
+ *
+ * Use this instead of calling MultiXactIdWait()
+ */
+static void
+MultiXactIdWaitExtended(Relation rel, ItemPointerData ctid,
+ const char *opr_name, MultiXactId multi,
+ MultiXactStatus status, int *remaining,
+ uint16 infomask)
+{
+ struct XactLockTableWaitLockInfo info;
+ ErrorContextCallback callback;
+
+ 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;
+
+ MultiXactIdWait(multi, status, remaining, infomask);
+
+ error_context_stack = callback.previous;
+}
+
+/*
* ConditionalMultiXactIdWait
* As above, but only lock if we can get the lock without blocking.
*
diff --git a/src/backend/access/nbtree/nbtinsert.c b/src/backend/access/nbtree/nbtinsert.c
index 5f7953f..d15af17 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);
+ XactLockTableWaitExtended(rel, itup->t_tid, "index insert for", xwait);
/* start over... */
_bt_freestack(stack);
goto top;
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 877d767..85c7b8c 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -2295,7 +2295,10 @@ IndexBuildHeapScan(Relation heapRelation,
* Must drop the lock on the buffer before we wait
*/
LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK);
- XactLockTableWait(xwait);
+ XactLockTableWaitExtended(heapRelation,
+ heapTuple->t_data->t_ctid,
+ "index the",
+ xwait);
goto recheck;
}
}
@@ -2341,7 +2344,10 @@ IndexBuildHeapScan(Relation heapRelation,
* Must drop the lock on the buffer before we wait
*/
LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK);
- XactLockTableWait(xwait);
+ XactLockTableWaitExtended(heapRelation,
+ heapTuple->t_data->t_ctid,
+ "index the",
+ xwait);
goto recheck;
}
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 9e3d879..e826b14 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);
+ XactLockTableWaitExtended(relation, tuple.t_data->t_ctid,
+ "recheck updated",
+ SnapshotDirty.xmax);
continue; /* loop back to repeat heap_fetch */
}
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index 46895b2..fdbcade 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);
+ XactLockTableWaitExtended(heap, tup->t_data->t_ctid,
+ "check exclusion constraint on", xwait);
goto retry;
}
diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c
index 4c61a6f..6ce2e69 100644
--- a/src/backend/storage/lmgr/lmgr.c
+++ b/src/backend/storage/lmgr/lmgr.c
@@ -19,11 +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"
/*
* RelationInitLockInfo
@@ -534,6 +535,103 @@ 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)
+{
+ struct XactLockTableWaitLockInfo *info = (struct 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 attempting to %s tuple (%u,%u) in relation "
+ "\"%s\".\"%s\" of database \"%s\"",
+ info->opr_name,
+ BlockIdGetBlockNumber(&(info->ctid.ip_blkid)),
+ info->ctid.ip_posid,
+ get_namespace_name(RelationGetNamespace(info->rel)),
+ RelationGetRelationName(info->rel),
+ get_database_name(info->rel->rd_node.dbNode));
+ else if (MyDatabaseId != InvalidOid)
+ errcontext("while attempting to %s tuple (%u,%u) "
+ "in database \"%s\"",
+ info->opr_name,
+ BlockIdGetBlockNumber(&(info->ctid.ip_blkid)),
+ info->ctid.ip_posid,
+ get_database_name(MyDatabaseId));
+ else
+ errcontext("while attempting to %s tuple (%u,%u)",
+ info->opr_name,
+ BlockIdGetBlockNumber(&(info->ctid.ip_blkid)),
+ info->ctid.ip_posid);
+ }
+ else
+ {
+ if (info->rel != NULL)
+ errcontext("while attempting to %s tuple in relation "
+ "\"%s\".\"%s\" of database \"%s\"",
+ info->opr_name,
+ get_namespace_name(RelationGetNamespace(info->rel)),
+ RelationGetRelationName(info->rel),
+ get_database_name(info->rel->rd_node.dbNode));
+ else if (MyDatabaseId != InvalidOid)
+ errcontext("while attempting to %s tuple in database \"%s\"",
+ info->opr_name,
+ get_database_name(MyDatabaseId));
+
+ /*
+ * 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
+ */
+ }
+}
+
+/*
+ * XactLockTableWaitExtended
+ * Sets up the error context callback for reporting more
+ * information of locked tuple.
+ *
+ * Use this instead of calling XactLockTableWait() to aquire
+ * transaction lock for tuple.
+ */
+void
+XactLockTableWaitExtended(Relation rel, ItemPointerData ctid,
+ const char *opr_name, TransactionId xid)
+{
+ struct XactLockTableWaitLockInfo info;
+ ErrorContextCallback callback;
+
+ 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;
+
+ XactLockTableWait(xid);
+
+ error_context_stack = callback.previous;
+}
+
+/*
* 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..8b7ff0a 100644
--- a/src/include/storage/lmgr.h
+++ b/src/include/storage/lmgr.h
@@ -18,6 +18,23 @@
#include "storage/itemptr.h"
#include "storage/lock.h"
#include "utils/rel.h"
+#include "access/htup.h"
+
+/*
+ * We fill this structure to provide useful information for transaction lock.
+ * XactLockTableWaitExtended/MultiXactIdWaitExtended will fill this struct
+ * before start waiting on transaction lock.
+ * 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.
+ */
+struct XactLockTableWaitLockInfo
+{
+ Relation rel;
+ ItemPointerData ctid;
+ const char *opr_name;
+};
extern void RelationInitLockInfo(Relation relation);
@@ -57,6 +74,10 @@ extern void XactLockTableDelete(TransactionId xid);
extern void XactLockTableWait(TransactionId xid);
extern bool ConditionalXactLockTableWait(TransactionId xid);
+extern void XactLockTableWaitExtended(Relation rel, ItemPointerData ctid,
+ const char *opr_name, 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);
pgpM1inr7Vx6B.pgp
Description: PGP signature
