On 2014-04-21 19:43:15 -0400, Andrew Dunstan wrote:
>
> On 04/21/2014 02:54 PM, Andres Freund wrote:
> >Hi,
> >
> >I spent the last two hours poking arounds in the environment Andrew
> >provided and I was able to reproduce the issue, find a assert to
> >reproduce it much faster and find a possible root cause.
>
>
> What's the assert that makes it happen faster? That might help a lot in
> constructing a self-contained test.
Assertion and *preliminary*, *hacky* fix attached.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
>From 40c09d8a85db137caf42c2dfe36adda63a381c75 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Tue, 22 Apr 2014 08:42:11 +0200
Subject: [PATCH 1/2] Error out when creating a multixact with more than one
updater.
---
src/backend/access/heap/heapam.c | 3 ---
src/backend/access/transam/multixact.c | 15 +++++++++++++++
src/include/access/multixact.h | 3 +++
3 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 2605f20..bd26b80 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -190,9 +190,6 @@ static const int MultiXactStatusLock[MaxMultiXactStatus + 1] =
/* Get the LockTupleMode for a given MultiXactStatus */
#define TUPLOCK_from_mxstatus(status) \
(MultiXactStatusLock[(status)])
-/* Get the is_update bit for a given MultiXactStatus */
-#define ISUPDATE_from_mxstatus(status) \
- ((status) > MultiXactStatusForUpdate)
/* ----------------------------------------------------------------
* heap support routines
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index d4ad678..4ff1e58 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -713,6 +713,21 @@ MultiXactIdCreateFromMembers(int nmembers, MultiXactMember *members)
return multi;
}
+ {
+ int i;
+ TransactionId update_xact = InvalidTransactionId;
+
+ for (i = 0; i < nmembers; i++)
+ {
+ /* Ignore lockers */
+ if (!ISUPDATE_from_mxstatus(members[i].status))
+ continue;
+ if (update_xact != InvalidTransactionId)
+ elog(ERROR, "new multixact has more than one updating member");
+ update_xact = members[i].xid;
+ }
+ }
+
/*
* Assign the MXID and offsets range to use, and make sure there is space
* in the OFFSETs and MEMBERs files. NB: this routine does
diff --git a/src/include/access/multixact.h b/src/include/access/multixact.h
index 9729f27..95ffe0e 100644
--- a/src/include/access/multixact.h
+++ b/src/include/access/multixact.h
@@ -48,6 +48,9 @@ typedef enum
#define MaxMultiXactStatus MultiXactStatusUpdate
+/* Get the is_update bit for a given MultiXactStatus */
+#define ISUPDATE_from_mxstatus(status) \
+ ((status) > MultiXactStatusForUpdate)
typedef struct MultiXactMember
{
--
1.8.5.rc2.dirty
>From ebd4f834b7cdccb9a9fe336dd981811e26bfc271 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Tue, 22 Apr 2014 08:42:44 +0200
Subject: [PATCH 2/2] preliminary fix for corruption issue.
---
src/backend/access/heap/heapam.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index bd26b80..9c075c4 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -2722,7 +2722,7 @@ l1:
* update this tuple before we get to this point. Check for xmax
* change, and start over if so.
*/
- if (!(tp.t_data->t_infomask & HEAP_XMAX_IS_MULTI) ||
+ if (tp.t_data->t_infomask != infomask ||
!TransactionIdEquals(HeapTupleHeaderGetRawXmax(tp.t_data),
xwait))
goto l1;
@@ -2748,7 +2748,7 @@ l1:
* other xact could update this tuple before we get to this point.
* Check for xmax change, and start over if so.
*/
- if ((tp.t_data->t_infomask & HEAP_XMAX_IS_MULTI) ||
+ if (tp.t_data->t_infomask != infomask ||
!TransactionIdEquals(HeapTupleHeaderGetRawXmax(tp.t_data),
xwait))
goto l1;
@@ -3275,7 +3275,7 @@ l2:
* update this tuple before we get to this point. Check for xmax
* change, and start over if so.
*/
- if (!(oldtup.t_data->t_infomask & HEAP_XMAX_IS_MULTI) ||
+ if (oldtup.t_data->t_infomask != infomask ||
!TransactionIdEquals(HeapTupleHeaderGetRawXmax(oldtup.t_data),
xwait))
goto l2;
@@ -3329,7 +3329,7 @@ l2:
* recheck the locker; if someone else changed the tuple while
* we weren't looking, start over.
*/
- if ((oldtup.t_data->t_infomask & HEAP_XMAX_IS_MULTI) ||
+ if (oldtup.t_data->t_infomask != infomask ||
!TransactionIdEquals(
HeapTupleHeaderGetRawXmax(oldtup.t_data),
xwait))
@@ -3350,7 +3350,7 @@ l2:
* some other xact could update this tuple before we get to
* this point. Check for xmax change, and start over if so.
*/
- if ((oldtup.t_data->t_infomask & HEAP_XMAX_IS_MULTI) ||
+ if (oldtup.t_data->t_infomask != infomask ||
!TransactionIdEquals(
HeapTupleHeaderGetRawXmax(oldtup.t_data),
xwait))
@@ -4354,7 +4354,7 @@ l3:
* over.
*/
LockBuffer(*buffer, BUFFER_LOCK_EXCLUSIVE);
- if (!(tuple->t_data->t_infomask & HEAP_XMAX_IS_MULTI) ||
+ if (tuple->t_data->t_infomask != infomask ||
!TransactionIdEquals(HeapTupleHeaderGetRawXmax(tuple->t_data),
xwait))
{
@@ -4373,7 +4373,7 @@ l3:
LockBuffer(*buffer, BUFFER_LOCK_EXCLUSIVE);
/* if the xmax changed in the meantime, start over */
- if ((tuple->t_data->t_infomask & HEAP_XMAX_IS_MULTI) ||
+ if (tuple->t_data->t_infomask != infomask ||
!TransactionIdEquals(
HeapTupleHeaderGetRawXmax(tuple->t_data),
xwait))
@@ -4441,7 +4441,7 @@ l3:
* could update this tuple before we get to this point. Check
* for xmax change, and start over if so.
*/
- if (!(tuple->t_data->t_infomask & HEAP_XMAX_IS_MULTI) ||
+ if (tuple->t_data->t_infomask != infomask ||
!TransactionIdEquals(
HeapTupleHeaderGetRawXmax(tuple->t_data),
xwait))
@@ -4497,7 +4497,7 @@ l3:
* some other xact could update this tuple before we get to
* this point. Check for xmax change, and start over if so.
*/
- if ((tuple->t_data->t_infomask & HEAP_XMAX_IS_MULTI) ||
+ if (tuple->t_data->t_infomask != infomask ||
!TransactionIdEquals(
HeapTupleHeaderGetRawXmax(tuple->t_data),
xwait))
--
1.8.5.rc2.dirty
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
