Robert Haas escribió:
> On Tue, Mar 18, 2014 at 12:00 AM, Amit Kapila <amit.kapil...@gmail.com> wrote:
> >> Therefore I think the only case worth considering here is when
> >> database/relation/TID are all defined. The other cases where there is
> >> partial information are dead code; and the case where there is nothing
> >> defined (such as the one in SnapBuildFindSnapshot) is already handled by
> >> simply not setting up a context at all.
> >
> > Right. So I think we should just keep one version of message.
>
> Well, if we're back to one version of the message, and I'm glad we
> are, can we go back to saying:
>
> CONTEXT: while updating tuple (0,2) in relation "public"."foo" of
> database "postgres"
That isn't easy. The callers would need to pass the whole message in
order for it to be translatable; and generating a format string in one
module and having the arguments be stapled on top in a separate module
doesn't seem a very good idea to me. Currently we have this:
/* wait for regular transaction to end */
XactLockTableWait(xwait, relation, &tp.t_data->t_ctid,
/* translator: string is XactLockTableWait operation
name */
"deleting tuple");
And if we go with what you propose, we would have this:
/* wait for regular transaction to end */
XactLockTableWait(xwait, relation, &tp.t_data->t_ctid,
"while updating tuple (%u,%u) in relation \"%s\"")
which doesn't seem an improvement to me.
Now another idea would be to pass a code or something; so callers would
do
XactLockTableWait(xwait, relation, TID, XLTW_OPER_UPDATE);
and the callback would select the appropriate message. Not really
excited about that, though.
In the current version of the patch, attached, I've reduced the callback
to this:
if (ItemPointerIsValid(info->ctid) && RelationIsValid(info->rel))
{
errcontext("while operating on tuple (%u,%u) in relation
\"%s\": %s",
ItemPointerGetBlockNumber(info->ctid),
ItemPointerGetOffsetNumber(info->ctid),
RelationGetRelationName(info->rel),
_(info->opr_name));
}
Note that:
1. the database name is gone, as discussed
2. the schema name is gone too, because it requires a syscache lookup
Now we can go back to having a schema name, but I think we would have to
add an IsTransactionState() check first or something like that. Or save
the schema name when setting up the callback, but this doesn't sound so
good (particularly considering that lock waits might end without
actually waiting at all, so this'd be wasted effort.)
If you have a better idea, I'm all ears.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
*** a/src/backend/access/heap/heapam.c
--- b/src/backend/access/heap/heapam.c
***************
*** 105,115 **** 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 XLogRecPtr log_heap_new_cid(Relation relation, HeapTuple tup);
static HeapTuple ExtractReplicaIdentity(Relation rel, HeapTuple tup, bool key_modified,
bool *copy);
--- 105,116 ----
uint16 *new_infomask2);
static TransactionId MultiXactIdGetUpdateXid(TransactionId xmax,
uint16 t_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,2721 **** l1:
if (infomask & HEAP_XMAX_IS_MULTI)
{
/* wait for multixact */
! MultiXactIdWait((MultiXactId) xwait, MultiXactStatusUpdate,
! NULL, infomask);
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
/*
--- 2715,2724 ----
if (infomask & HEAP_XMAX_IS_MULTI)
{
/* wait for multixact */
! MultiXactIdWait((MultiXactId) xwait, MultiXactStatusUpdate, infomask,
! /* translator: string is XactLockTableWait operation name */
! relation, &tp.t_data->t_ctid, "deleting tuple",
! NULL);
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
/*
***************
*** 2741,2747 **** l1:
else
{
/* wait for regular transaction to end */
! XactLockTableWait(xwait);
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
/*
--- 2744,2752 ----
else
{
/* wait for regular transaction to end */
! XactLockTableWait(xwait, relation, &tp.t_data->t_ctid,
! /* translator: string is XactLockTableWait operation name */
! "deleting tuple");
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
/*
***************
*** 3266,3273 **** l2:
int remain;
/* wait for multixact */
! MultiXactIdWait((MultiXactId) xwait, mxact_status, &remain,
! infomask);
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
/*
--- 3271,3280 ----
int remain;
/* wait for multixact */
! MultiXactIdWait((MultiXactId) xwait, mxact_status, infomask,
! /* translator: string is XactLockTableWait operation name */
! relation, &oldtup.t_data->t_ctid, "updating tuple",
! &remain);
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
/*
***************
*** 3341,3347 **** l2:
else
{
/* wait for regular transaction to end */
! XactLockTableWait(xwait);
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
/*
--- 3348,3356 ----
else
{
/* wait for regular transaction to end */
! XactLockTableWait(xwait, relation, &oldtup.t_data->t_ctid,
! /* translator: string is XactLockTableWait operation name */
! "updating tuple");
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
/*
***************
*** 4402,4415 **** l3:
if (nowait)
{
if (!ConditionalMultiXactIdWait((MultiXactId) xwait,
! status, NULL, infomask))
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);
/* if there are updates, follow the update chain */
if (follow_updates &&
--- 4411,4430 ----
if (nowait)
{
if (!ConditionalMultiXactIdWait((MultiXactId) xwait,
! status, infomask, relation,
! &tuple->t_data->t_ctid,
! /* translator: string is XactLockTableWait operation name */
! "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, infomask,
! relation, &tuple->t_data->t_ctid,
! /* translator: string is XactLockTableWait operation name */
! "locking tuple", NULL);
/* if there are updates, follow the update chain */
if (follow_updates &&
***************
*** 4464,4470 **** l3:
RelationGetRelationName(relation))));
}
else
! XactLockTableWait(xwait);
/* if there are updates, follow the update chain */
if (follow_updates &&
--- 4479,4487 ----
RelationGetRelationName(relation))));
}
else
! XactLockTableWait(xwait, relation, &tuple->t_data->t_ctid,
! /* translator: string is XactLockTableWait operation name */
! "locking tuple");
/* if there are updates, follow the update chain */
if (follow_updates &&
***************
*** 5151,5157 **** l4:
if (needwait)
{
LockBuffer(buf, BUFFER_LOCK_UNLOCK);
! XactLockTableWait(members[i].xid);
pfree(members);
goto l4;
}
--- 5168,5178 ----
if (needwait)
{
LockBuffer(buf, BUFFER_LOCK_UNLOCK);
! XactLockTableWait(members[i].xid, rel,
! &mytup.t_data->t_ctid,
! /*---
! translator: string is XactLockTableWait operation name */
! "locking updated version of tuple");
pfree(members);
goto l4;
}
***************
*** 5211,5217 **** l4:
if (needwait)
{
LockBuffer(buf, BUFFER_LOCK_UNLOCK);
! XactLockTableWait(rawxmax);
goto l4;
}
if (res != HeapTupleMayBeUpdated)
--- 5232,5241 ----
if (needwait)
{
LockBuffer(buf, BUFFER_LOCK_UNLOCK);
! XactLockTableWait(rawxmax, rel, &mytup.t_data->t_ctid,
! /*---
! translator: string is XactLockTableWait operation name */
! "locking updated version of tuple");
goto l4;
}
if (res != HeapTupleMayBeUpdated)
***************
*** 6076,6081 **** HeapTupleGetUpdateXid(HeapTupleHeader tuple)
--- 6100,6114 ----
* 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,6099 **** HeapTupleGetUpdateXid(HeapTupleHeader tuple)
*/
static bool
Do_MultiXactIdWait(MultiXactId multi, MultiXactStatus status,
! int *remaining, uint16 infomask, bool nowait)
{
bool allow_old;
bool result = true;
--- 6126,6134 ----
*/
static bool
Do_MultiXactIdWait(MultiXactId multi, MultiXactStatus status,
! uint16 infomask, bool nowait,
! Relation rel, ItemPointer ctid, const char *opr,
! int *remaining)
{
bool allow_old;
bool result = true;
***************
*** 6130,6135 **** Do_MultiXactIdWait(MultiXactId multi, MultiXactStatus status,
--- 6165,6176 ----
/*
* 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,6144 **** Do_MultiXactIdWait(MultiXactId multi, MultiXactStatus status,
break;
}
else
! XactLockTableWait(memxid);
}
pfree(members);
--- 6179,6185 ----
break;
}
else
! XactLockTableWait(memxid, rel, ctid, opr);
}
pfree(members);
***************
*** 6159,6171 **** 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)
{
! Do_MultiXactIdWait(multi, status, remaining, infomask, false);
}
/*
--- 6200,6213 ----
*
* 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, uint16 infomask,
! Relation rel, ItemPointer ctid, const char *opr,
! int *remaining)
{
! (void) Do_MultiXactIdWait(multi, status, infomask, false,
! rel, ctid, opr, remaining);
}
/*
***************
*** 6183,6191 **** MultiXactIdWait(MultiXactId multi, MultiXactStatus status,
*/
static bool
ConditionalMultiXactIdWait(MultiXactId multi, MultiXactStatus status,
! int *remaining, uint16 infomask)
{
! return Do_MultiXactIdWait(multi, status, remaining, infomask, true);
}
/*
--- 6225,6235 ----
*/
static bool
ConditionalMultiXactIdWait(MultiXactId multi, MultiXactStatus status,
! uint16 infomask, Relation rel, ItemPointer ctid,
! const char *opr, int *remaining)
{
! return Do_MultiXactIdWait(multi, status, infomask, true,
! rel, ctid, opr, remaining);
}
/*
*** a/src/backend/access/nbtree/nbtinsert.c
--- b/src/backend/access/nbtree/nbtinsert.c
***************
*** 164,170 **** top:
{
/* Have to wait for the other guy ... */
_bt_relbuf(rel, buf);
! XactLockTableWait(xwait);
/* start over... */
_bt_freestack(stack);
goto top;
--- 164,171 ----
{
/* Have to wait for the other guy ... */
_bt_relbuf(rel, buf);
! /* translator: string is XactLockTableWait operation name */
! XactLockTableWait(xwait, rel, &itup->t_tid, "inserting index tuple");
/* start over... */
_bt_freestack(stack);
goto top;
*** a/src/backend/catalog/index.c
--- b/src/backend/catalog/index.c
***************
*** 2295,2301 **** IndexBuildHeapScan(Relation heapRelation,
* Must drop the lock on the buffer before we wait
*/
LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK);
! XactLockTableWait(xwait);
goto recheck;
}
}
--- 2295,2305 ----
* Must drop the lock on the buffer before we wait
*/
LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK);
! XactLockTableWait(xwait, heapRelation,
! &heapTuple->t_data->t_ctid,
! /*---
! translator: string is XactLockTableWait operation name */
! "creating unique index tuple");
goto recheck;
}
}
***************
*** 2341,2347 **** IndexBuildHeapScan(Relation heapRelation,
* Must drop the lock on the buffer before we wait
*/
LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK);
! XactLockTableWait(xwait);
goto recheck;
}
--- 2345,2355 ----
* Must drop the lock on the buffer before we wait
*/
LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK);
! XactLockTableWait(xwait, heapRelation,
! &heapTuple->t_data->t_ctid,
! /*---
! translator: string is XactLockTableWait operation name */
! "creating unique index tuple");
goto recheck;
}
*** a/src/backend/executor/execMain.c
--- b/src/backend/executor/execMain.c
***************
*** 1982,1988 **** EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
if (TransactionIdIsValid(SnapshotDirty.xmax))
{
ReleaseBuffer(buffer);
! XactLockTableWait(SnapshotDirty.xmax);
continue; /* loop back to repeat heap_fetch */
}
--- 1982,1991 ----
if (TransactionIdIsValid(SnapshotDirty.xmax))
{
ReleaseBuffer(buffer);
! XactLockTableWait(SnapshotDirty.xmax,
! relation, &tuple.t_data->t_ctid,
! /* translator: string is XactLockTableWait operation name */
! "rechecking updated version of tuple");
continue; /* loop back to repeat heap_fetch */
}
*** a/src/backend/executor/execUtils.c
--- b/src/backend/executor/execUtils.c
***************
*** 105,111 **** CreateExecutorState(void)
* Initialize all fields of the Executor State structure
*/
estate->es_direction = ForwardScanDirection;
! estate->es_snapshot = InvalidSnapshot; /* caller must initialize this */
estate->es_crosscheck_snapshot = InvalidSnapshot; /* no crosscheck */
estate->es_range_table = NIL;
estate->es_plannedstmt = NULL;
--- 105,111 ----
* Initialize all fields of the Executor State structure
*/
estate->es_direction = ForwardScanDirection;
! estate->es_snapshot = InvalidSnapshot; /* caller must initialize this */
estate->es_crosscheck_snapshot = InvalidSnapshot; /* no crosscheck */
estate->es_range_table = NIL;
estate->es_plannedstmt = NULL;
***************
*** 1307,1313 **** retry:
if (TransactionIdIsValid(xwait))
{
index_endscan(index_scan);
! XactLockTableWait(xwait);
goto retry;
}
--- 1307,1315 ----
if (TransactionIdIsValid(xwait))
{
index_endscan(index_scan);
! XactLockTableWait(xwait, heap, &tup->t_data->t_ctid,
! /* translator: string is XactLockTableWait operation name */
! "rechecking exclusion constraint tuple");
goto retry;
}
*** a/src/backend/replication/logical/snapbuild.c
--- b/src/backend/replication/logical/snapbuild.c
***************
*** 1343,1349 **** SnapBuildFindSnapshot(SnapBuild *builder, XLogRecPtr lsn, xl_running_xacts *runn
if (TransactionIdIsCurrentTransactionId(xid))
elog(ERROR, "waiting for ourselves");
! XactLockTableWait(xid);
}
/* nothing could have built up so far, so don't perform cleanup */
--- 1343,1349 ----
if (TransactionIdIsCurrentTransactionId(xid))
elog(ERROR, "waiting for ourselves");
! XactLockTableWait(xid, NULL, NULL, NULL);
}
/* nothing could have built up so far, so don't perform cleanup */
*** a/src/backend/storage/lmgr/lmgr.c
--- b/src/backend/storage/lmgr/lmgr.c
***************
*** 25,30 ****
--- 25,32 ----
#include "utils/inval.h"
+ static void XactLockTableWaitErrorContextCallback(void *arg);
+
/*
* RelationInitLockInfo
* Initializes the lock information in a relation descriptor.
***************
*** 471,477 **** XactLockTableDelete(TransactionId xid)
/*
* XactLockTableWait
*
! * Wait for the specified transaction to commit or abort.
*
* 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.
--- 473,481 ----
/*
* XactLockTableWait
*
! * Wait for the specified transaction to commit or abort. If we're given a
! * relation, tuple id and operation name, an error context callback is set up.
! * If opr_name is passed as NULL, no error context callback is set up.
*
* 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,489 **** XactLockTableDelete(TransactionId xid)
* and if so wait for its parent.
*/
void
! XactLockTableWait(TransactionId xid)
{
LOCKTAG tag;
for (;;)
{
--- 485,518 ----
* and if so wait for its parent.
*/
void
! 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.
+ *
+ * Note: we avoid an empty opr_name also, because it might be used to
+ * lookup a translated entry later.
+ */
+ if (opr_name != NULL && opr_name[0] != '\0')
+ {
+ Assert(RelationIsValid(rel));
+ Assert(ItemPointerIsValid(ctid));
+
+ 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,505 **** XactLockTableWait(TransactionId xid)
--- 529,537 ----
break;
xid = SubTransGetParent(xid);
}
+
+ if (opr_name != NULL)
+ error_context_stack = callback.previous;
}
/*
***************
*** 534,539 **** ConditionalXactLockTableWait(TransactionId xid)
--- 566,594 ----
}
/*
+ * XactLockTableWaitErrorContextCallback
+ * Error context callback for transaction lock waits.
+ */
+ static void
+ XactLockTableWaitErrorContextCallback(void *arg)
+ {
+ XactLockTableWaitLockInfo *info = (XactLockTableWaitLockInfo *) arg;
+
+ /*
+ * We would like to print schema name too, but that would require a
+ * syscache lookup.
+ */
+ if (ItemPointerIsValid(info->ctid) && RelationIsValid(info->rel))
+ {
+ errcontext("while operating on tuple (%u,%u) in relation \"%s\": %s",
+ ItemPointerGetBlockNumber(info->ctid),
+ ItemPointerGetOffsetNumber(info->ctid),
+ RelationGetRelationName(info->rel),
+ _(info->opr_name));
+ }
+ }
+
+ /*
* WaitForLockersMultiple
* Wait until no transaction holds locks that conflict with the given
* locktags at the given lockmode.
*** a/src/include/storage/lmgr.h
--- b/src/include/storage/lmgr.h
***************
*** 20,25 ****
--- 20,41 ----
#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,60 **** 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 bool ConditionalXactLockTableWait(TransactionId xid);
/* Lock VXIDs, specified by conflicting locktags */
--- 70,77 ----
/* 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, Relation rel,
! ItemPointer ctid, const char *opr_name);
extern bool ConditionalXactLockTableWait(TransactionId xid);
/* Lock VXIDs, specified by conflicting locktags */
*** a/src/nls-global.mk
--- b/src/nls-global.mk
***************
*** 57,63 **** BACKEND_COMMON_GETTEXT_TRIGGERS = \
errmsg errmsg_plural:1,2 \
errdetail errdetail_log errdetail_plural:1,2 \
errhint \
! errcontext
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 \
--- 57,66 ----
errmsg errmsg_plural:1,2 \
errdetail errdetail_log errdetail_plural:1,2 \
errhint \
! 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
