Re: foreign key locks, 2nd attempt

From: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Noah Misch <noah(at)leadboat(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: foreign key locks, 2nd attempt
Date: 2012-02-13 22:16:58
Message-ID: 1329170123-sup-322@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Excerpts from Alvaro Herrera's message of mar ene 31 20:55:19 -0300 2012:
> Excerpts from Robert Haas's message of mar ene 31 14:12:10 -0300 2012:
> >
> > On Tue, Jan 31, 2012 at 11:58 AM, Alvaro Herrera
> > <alvherre(at)commandprompt(dot)com> wrote:
> > > Well, we're already storing a multixact in Xmax, so it's not like we
> > > don't assume that we can store multis in space normally reserved for
> > > Xids.  What I've been wondering is not how ugly it is, but rather of the
> > > fact that we're bloating pg_class some more.
> >
> > I don't think another 4 bytes in pg_class is that big a deal. We
> > don't do relcache rebuilds frequently enough for that to really matter
> > much. The bigger cost of this patch seems to me to be that we're
> > going to have to carry around multi-xact IDs for a long time, and
> > probably fsync and/or WAL-log them moreso than now. I'm not sure how
> > much you've worried about that, but a new column in pg_class seems
> > like line noise by comparison.
>
> I'm not too worried by either fsyncing or WAL logging, because those
> costs are only going to be paid when a multixact is used at all; if we
> avoid having to wait for an arbitrary length of time at some point, then
> it doesn't matter than we some things are bit slower. I worry about a
> new pg_class column because it's going to be paid by everyone, whether
> they use multixacts or not.
>
> But, having never heard anybody stand against this proposal, I'll go do
> that.

Okay, so this patch fixes the truncation and wraparound issues through a
mechanism much like pg_clog's: it keeps track of the oldest possibly
existing multis on each and every table, and then during tuple freezing
those are removed. I also took the liberty to make the code remove
multis altogether (i.e. resetting the IS_MULTI hint bit) when only the
update remains and lockers are all gone.

I also cleaned up the code in heapam so that there's a couple of tables
mapping MultiXactStatus to LockTupleMode and back, and to heavyweight
lock modes (the older patches used functions to do this, which was
pretty ugly). I had to add a little helper function to lock.c to make
this work. I made a rather large bunch of other minor changes to close
minor bugs here and there.

Docs have been added, as have new tests for the isolation harness, which
I've ensured pass in both read committed and serializable modes; WAL
logging for locking updated versions of a tuple, when an old one is
locked due to an old snapshot, was also added; there's plenty of room
for growth in the MultiXact flag bits; the bit that made tables with no
keys lock the entire row all the time was removed; multiple places in
code comments were cleaned up that referred to this feature as "FOR KEY
LOCK" and ensured that it also mentions FOR KEY UPDATE; the pg_rowlocks,
pageinspect, pg_controldata, pg_resetxlog utilities have been updated.

All in all, I think this is in pretty much final shape. Only pg_upgrade
bits are still missing. If sharp eyes could give this a critical look
and knuckle-cracking testers could give it a spin, that would be
helpful.

contrib/pageinspect/heapfuncs.c | 2 +-
contrib/pgrowlocks/Makefile | 2 +-
contrib/pgrowlocks/pgrowlocks--1.0--1.1.sql | 17 +
contrib/pgrowlocks/pgrowlocks--1.0.sql | 15 -
contrib/pgrowlocks/pgrowlocks--1.1.sql | 15 +
contrib/pgrowlocks/pgrowlocks.c | 159 ++-
contrib/pgrowlocks/pgrowlocks.control | 2 +-
doc/src/sgml/pgrowlocks.sgml | 14 +-
doc/src/sgml/ref/select.sgml | 137 +-
src/backend/access/common/heaptuple.c | 2 +-
src/backend/access/heap/heapam.c | 1841 ++++++++++++++++----
src/backend/access/heap/pruneheap.c | 10 +-
src/backend/access/heap/rewriteheap.c | 16 +-
src/backend/access/transam/README | 6 +-
src/backend/access/transam/multixact.c | 1093 ++++++++----
src/backend/access/transam/varsup.c | 2 +
src/backend/access/transam/xact.c | 3 -
src/backend/access/transam/xlog.c | 19 +-
src/backend/catalog/heap.c | 14 +-
src/backend/catalog/index.c | 8 +-
src/backend/commands/analyze.c | 10 +-
src/backend/commands/cluster.c | 36 +-
src/backend/commands/dbcommands.c | 15 +-
src/backend/commands/sequence.c | 8 +-
src/backend/commands/tablecmds.c | 11 +-
src/backend/commands/trigger.c | 2 +-
src/backend/commands/vacuum.c | 92 +-
src/backend/commands/vacuumlazy.c | 23 +-
src/backend/executor/execMain.c | 14 +-
src/backend/executor/nodeLockRows.c | 23 +-
src/backend/nodes/copyfuncs.c | 4 +-
src/backend/nodes/equalfuncs.c | 4 +-
src/backend/nodes/outfuncs.c | 4 +-
src/backend/nodes/readfuncs.c | 2 +-
src/backend/optimizer/plan/initsplan.c | 6 +-
src/backend/optimizer/plan/planner.c | 39 +-
src/backend/parser/analyze.c | 58 +-
src/backend/parser/gram.y | 20 +-
src/backend/postmaster/autovacuum.c | 45 +-
src/backend/rewrite/rewriteHandler.c | 32 +-
src/backend/storage/lmgr/lock.c | 13 +
src/backend/storage/lmgr/predicate.c | 4 +-
src/backend/tcop/utility.c | 46 +-
src/backend/utils/adt/ri_triggers.c | 41 +-
src/backend/utils/adt/ruleutils.c | 30 +-
src/backend/utils/cache/relcache.c | 29 +-
src/backend/utils/time/combocid.c | 5 +-
src/backend/utils/time/tqual.c | 411 ++++-
src/bin/pg_controldata/pg_controldata.c | 4 +
src/bin/pg_resetxlog/pg_resetxlog.c | 17 +
src/include/access/heapam.h | 21 +-
src/include/access/htup.h | 85 +-
src/include/access/multixact.h | 65 +-
src/include/access/rewriteheap.h | 2 +-
src/include/access/xlog.h | 2 +
src/include/catalog/pg_class.h | 24 +-
src/include/catalog/pg_control.h | 2 +
src/include/catalog/pg_database.h | 10 +-
src/include/commands/cluster.h | 3 +-
src/include/commands/vacuum.h | 6 +-
src/include/nodes/execnodes.h | 8 +-
src/include/nodes/parsenodes.h | 36 +-
src/include/nodes/plannodes.h | 12 +-
src/include/parser/analyze.h | 2 +-
src/include/postgres.h | 7 +
src/include/storage/lock.h | 1 +
src/include/utils/rel.h | 1 +
src/include/utils/relcache.h | 4 +-
src/test/isolation/expected/aborted-keyrevoke.out | 276 +++
.../isolation/expected/aborted-keyrevoke_2.out | 278 +++
.../isolation/expected/delete-abort-savept-2.out | 76 +
.../isolation/expected/delete-abort-savept.out | 111 ++
src/test/isolation/expected/fk-contention.out | 3 +-
src/test/isolation/expected/fk-deadlock.out | 34 +-
src/test/isolation/expected/fk-deadlock2.out | 68 +-
src/test/isolation/expected/fk-deadlock2_1.out | 75 +-
src/test/isolation/expected/fk-deadlock2_2.out | 105 ++
src/test/isolation/expected/fk-deadlock_1.out | 44 +-
src/test/isolation/expected/fk-deadlock_2.out | 67 +
src/test/isolation/expected/fk-delete-insert.out | 41 +
src/test/isolation/expected/lock-update-delete.out | 65 +
.../isolation/expected/lock-update-traversal.out | 18 +
src/test/isolation/isolation_schedule | 5 +
src/test/isolation/isolationtester.c | 1 +
src/test/isolation/specs/aborted-keyrevoke.spec | 31 +
.../isolation/specs/delete-abort-savept-2.spec | 34 +
src/test/isolation/specs/delete-abort-savept.spec | 52 +
src/test/isolation/specs/fk-deadlock2.spec | 16 +-
src/test/isolation/specs/lock-update-delete.spec | 38 +
.../isolation/specs/lock-update-traversal.spec | 32 +
90 files changed, 4855 insertions(+), 1331 deletions(-)

--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Attachment Content-Type Size
fklocks-9.patch.gz application/x-gzip 82.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jan Urbański 2012-02-13 22:40:55 Re: pl/python long-lived allocations in datum->dict transformation
Previous Message Peter Eisentraut 2012-02-13 21:25:59 Re: psql tab completion for SELECT