From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Erik Rijkers <er@xs4all.nl>
Cc: Kevin Grittner <kgrittn@mail.com>
Cc: pgsql-hackers@postgresql.org, Andres Freund <andres@2ndquadrant.com>
Subject: Re: foreign key locks
Date: Fri, 18 Jan 2013 17:02:04 -0300
Message-ID: <20130118200204.GH4063@alvh.no-ip.org> (raw)
In-Reply-To: <20130110210040.GB4110@alvh.no-ip.org>
References: <20121129152326.69330@gmx.com>
<20121129160220.GA4942@alvh.no-ip.org>
<20121129161654.GB4942@alvh.no-ip.org>
<b1a78c715f9195e6f96546c8e723afd8.squirrel@webmail.xs4all.nl>
<20121228204847.GD4150@alvh.no-ip.org>
<20130110210040.GB4110@alvh.no-ip.org>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
Alvaro Herrera wrote:
> Here's version 28 of this patch. The only substantive change here from
> v26 is that I've made GetTupleForTrigger() use either LockTupleExclusive
> or LockTupleNoKeyExclusive, depending on whether the key columns are
> being modified by the update. This needs to go through EvalPlanQual, so
> that function is now getting the lock mode as a parameter instead of
> hardcoded LockTupleExclusive. (All other users of GetTupleForTrigger
> still use LockTupleExclusive, so there's no change for anybody other
> than FOR EACH ROW BEFORE UPDATE triggers).
>
> At this point, I think I've done all I wanted to do in connection with
> this patch, and I intend to commit. I think this is a good time to get
> it committed, so that people can play with it more extensively and
> report any breakage I might have caused before we even get into beta.
While trying this out this morning I noticed a bug in the XLog code:
trying to lock the updated version of a tuple when the page that
contains the updated version requires a backup block, would cause this:
PANIC: invalid xlog record length 0
The reason is that there is an (unknown to me) rule that there must be
some data not associated with a buffer:
/*
* NOTE: We disallow len == 0 because it provides a useful bit of extra
* error checking in ReadRecord. This means that all callers of
* XLogInsert must supply at least some not-in-a-buffer data. [...]
*/
This seems pretty strange to me. And having the rule be spelled out
only in a comment within XLogInsert and not at its top, and not nearby
the XLogRecData struct definition either, seems pretty strange to me.
I wonder how come every PG hacker except me knows this.
For the curious, I attach an isolationtester spec file I used to
reproduce the problem (and verify the fix).
To fix the crash I needed to do this:
inline patch 1: src/backend/access/heap/heapam.c (+12/-7 lines) download inline-1.patchdiff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.cindex 99fced1..9762890 100644--- a/src/backend/access/heap/heapam.c+++ b/src/backend/access/heap/heapam.c@@ -4838,7 +4838,7 @@ l4:
{
xl_heap_lock_updated xlrec;
XLogRecPtr recptr;
- XLogRecData rdata;+ XLogRecData rdata[2];
Page page = BufferGetPage(buf);
xlrec.target.node = rel->rd_node;
@@ -4846,13 +4846,18 @@ l4:
xlrec.xmax = new_xmax;
xlrec.infobits_set = compute_infobits(new_infomask, new_infomask2);
- rdata.data = (char *) &xlrec;- rdata.len = SizeOfHeapLockUpdated;- rdata.buffer = buf;- rdata.buffer_std = true;- rdata.next = NULL;+ rdata[0].data = (char *) &xlrec;+ rdata[0].len = SizeOfHeapLockUpdated;+ rdata[0].buffer = InvalidBuffer;+ rdata[0].next = &(rdata[1]);++ rdata[1].data = NULL;+ rdata[1].len = 0;+ rdata[1].buffer = buf;+ rdata[1].buffer_std = true;+ rdata[1].next = NULL;- recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_LOCK_UPDATED, &rdata);+ recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_LOCK_UPDATED, rdata);
PageSetLSN(page, recptr);
PageSetTLI(page, ThisTimeLineID);
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackersAttachments:xlog-lock-updated.spec (text/plain, 621B) download
setup
{
CREATE TABLE foo (
key serial PRIMARY KEY,
value int
);
INSERT INTO foo SELECT value FROM generate_series(1, 226) AS value;
}
teardown
{
DROP TABLE foo;
}
session "s1"
step "s1b" { BEGIN ISOLATION LEVEL REPEATABLE READ; }
step "s1s" { SELECT * FROM foo LIMIT 0; } # obtain snapshot
step "s1l" { SELECT * FROM foo WHERE key = 1 FOR KEY SHARE; } # obtain lock
step "s1c" { COMMIT; }
session "s2"
step "s2b" { BEGIN; }
step "s2u" { UPDATE foo SET value = 2 WHERE key = 1; }
step "s2c" { COMMIT; }
session "s3"
step "s3ck" { CHECKPOINT; }
permutation "s1b" "s1s" "s2b" "s2u" "s3ck" "s1l" "s1c" "s2c"
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: alvherre@2ndquadrant.com, er@xs4all.nl, kgrittn@mail.com, andres@2ndquadrant.com
Subject: Re: foreign key locks
In-Reply-To: <20130118200204.GH4063@alvh.no-ip.org>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox