Re: foreign key locks, 2nd attempt - Mailing list pgsql-hackers

This version of the patch fixes most of the problems pointed out by this
review.  There are a bunch of relatively minor items that need
addressing yet, but I wanted to throw this out in case anyone is
interested in giving this some testing or more feedback.

The biggest item remaining is the point you raised about multixactid
wraparound.  This is closely related to multixact truncation and the way
checkpoints are to be handled.  If we think that MultiXactId wraparound
is possible, and we need to involve autovacuum to keep it at bay, then I
think the only way to make that work is to add another column to
pg_class so that each table's oldest multixact is tracked, same as we do
with relfrozenxid for Xids.  If we do that, I think we can do away with
most of the MultiXactTruncate junk I added -- things would become a lot
simpler.  The cost would be bloating pg_class a bit more.  Are we okay
with paying that cost?  I asked this question some months ago and I
decided that I would not add the column, but I am starting to lean the
other way.  I would like some opinions on this.

You asked two questions about WAL-logging locks: one was about the level
of detail we log for each lock we grab; the other was about
heap_xlog_update logging the right info.  AFAICS, the main thing that
makes detailed WAL logging necessary is hot standbys.  That is, the
standby must have all the locking info so that concurrent transactions
are similarly locked as in the master ... or am I wrong in that?  (ISTM
this means I need to fix heap_xlog_update so that it faithfully
registers the lock info we're storing, not just the current Xid).

You also asked about heap_update and TOAST; in particular, do we need to
re-check locking info after we've unlocked the buffer for toasting and
finding free space?  I believe the current version handles this, but I
haven't tested it.


Some open questions:

* Do we need some extra flag bits for each multi?

* how to deal with heap_update in the 'nowait' case?

* multixact.c cache
  Do we need some updates to that?

* multis with multiple members per Xid ??
  >   * MultiXactIdCreate
  >   *        Construct a MultiXactId representing two TransactionIds.
  >   *
  > - * The two XIDs must be different.
  > + * The two XIDs must be different, or be requesting different lock modes.

  Why is it not sufficient to store the strongest type for a particular xid?
  In this version, I've moved MultiXactIdWait to heapam.c.  This makes
  multixact largely unaware of the meaning of the flag bits stored with
  each multi.  I believe we can fix this problem, but not at this level,
  but rather in heap_lock_tuple.

* Columns that are part of the key
  Noah thinks the set of columns should only consider those actually referenced
  by keys, not those that *could* be referenced.

  Also, in a table without columns, are all columns part of the key, or is the
  key the empty set?  I changed HeapSatisfiesHOTUpdate but that seems arbitrary.



Need more code changes for the following:

* pg_upgrade issues are still open
  There are two things here.  One is what to do when migrating from an old
  version that only has HEAP_XMAX_SHARED_LOCK into a new one.  The other is
  what we need to do from 9.2 into the future (need to copy pg_multixact
  contents).

* export FOR KEY UPDATE lock mode in SQL

* Ensure that MultiXactIdIsValid is sane.

* heap_lock_updated_tuple needs WAL logging.


git diff --stat:

 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                    |  133 ++-
 contrib/pgrowlocks/pgrowlocks.control              |    2 +-
 doc/src/sgml/pgrowlocks.sgml                       |   14 +-
 doc/src/sgml/ref/select.sgml                       |  117 +-
 src/backend/access/common/heaptuple.c              |    2 +-
 src/backend/access/heap/heapam.c                   | 1524 ++++++++++++++++----
 src/backend/access/heap/pruneheap.c                |   10 +-
 src/backend/access/heap/rewriteheap.c              |    6 +-
 src/backend/access/transam/multixact.c             | 1436 ++++++++++---------
 src/backend/access/transam/twophase_rmgr.c         |    4 -
 src/backend/access/transam/xact.c                  |    3 -
 src/backend/access/transam/xlog.c                  |   14 +-
 src/backend/catalog/index.c                        |    4 +-
 src/backend/commands/analyze.c                     |    3 +-
 src/backend/commands/cluster.c                     |    2 +-
 src/backend/commands/sequence.c                    |    3 +-
 src/backend/commands/trigger.c                     |    2 +-
 src/backend/commands/vacuum.c                      |    2 +-
 src/backend/executor/execMain.c                    |    7 +-
 src/backend/executor/nodeLockRows.c                |   20 +-
 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               |   24 +-
 src/backend/parser/analyze.c                       |   24 +-
 src/backend/parser/gram.y                          |   12 +-
 src/backend/rewrite/rewriteHandler.c               |   26 +-
 src/backend/storage/lmgr/predicate.c               |    4 +-
 src/backend/tcop/utility.c                         |   40 +-
 src/backend/utils/adt/ri_triggers.c                |   41 +-
 src/backend/utils/adt/ruleutils.c                  |   26 +-
 src/backend/utils/cache/relcache.c                 |   23 +-
 src/backend/utils/time/combocid.c                  |    5 +-
 src/backend/utils/time/tqual.c                     |  356 ++++-
 src/bin/pg_resetxlog/pg_resetxlog.c                |   33 +-
 src/include/access/heapam.h                        |   16 +-
 src/include/access/htup.h                          |   66 +-
 src/include/access/multixact.h                     |   68 +-
 src/include/access/twophase_rmgr.h                 |    3 +-
 src/include/catalog/pg_control.h                   |    6 +-
 src/include/nodes/execnodes.h                      |    8 +-
 src/include/nodes/parsenodes.h                     |   34 +-
 src/include/nodes/plannodes.h                      |    9 +-
 src/include/parser/analyze.h                       |    2 +-
 src/include/utils/rel.h                            |    1 +
 src/include/utils/relcache.h                       |    2 +-
 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              |    2 +
 src/test/isolation/isolationtester.c               |    3 +-
 src/test/isolation/specs/fk-deadlock2.spec         |   16 +-
 src/test/isolation/specs/lock-update-delete.spec   |   38 +
 .../isolation/specs/lock-update-traversal.spec     |   32 +
 68 files changed, 3323 insertions(+), 1496 deletions(-)

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