On 17.12.2010 18:44, Florian Pflug wrote:
On Dec17, 2010, at 16:49 , Heikki Linnakangas wrote:
On 15.12.2010 16:20, Florian Pflug wrote:
On Dec14, 2010, at 15:01 , Robert Haas wrote:
On Tue, Dec 14, 2010 at 7:51 AM, Florian Pflug<f...@phlo.org> wrote:
- serializable lock consistency - I am fairly certain this needs
rebasing. I don't have time to deal with it right away. That sucks,
because I think this is a really important change.
I can try to find some time to update the patch if it suffers from bit-rot.
Would that help?
Yes!
I've rebased the patch to the current HEAD, and re-run my FK concurrency test
suite,
available from https://github.com/fgp/fk_concurrency, to verify that things
still work.
I've also asserts to the callers of heap_{update,delete,lock_tuple} to verify
(and document)
that update_xmax may only be InvalidTransactionId if a lockcheck_snapshot is
passed to
heap_{update,delete,lock_tuple}.
Finally, I've improved the explanation in src/backend/executor/README of how
row locks and
REPEATABLE READ transactions interact, and tried to state the guarantees
provided by
FOR SHARE and FOR UPDATE locks more precisely.
I've published my work to
https://github.com/fgp/postgres/tree/serializable_lock_consistency,
and attached an updated patch. I'd be happy to give write access to that GIT
repository
to anyone who wants to help getting this committed.
Here's some typo& style fixes for that, also available at
git://git.postgresql.org/git/users/heikki/postgres.git.
Thanks! FYI, I've pulled these into
https://github.com/fgp/postgres/tree/serializable_lock_consistency
I think this patch is in pretty good shape now.
The one thing I'm not too happy with is the API for
heap_update/delete/lock_tuple. The return value is:
* Normal, successful return value is HeapTupleMayBeUpdated, which
* actually means we *did* update it. Failure return codes are
* HeapTupleSelfUpdated, HeapTupleUpdated, or HeapTupleBeingUpdated
* (the last only possible if wait == false).
And:
* In the failure cases, the routine returns the tuple's t_ctid and t_max
* in ctid and update_xmax.
* If ctid is the same as t_self and update_xmax a valid transaction id,
* the tuple was deleted.
* If ctid differs from t_self, the tuple was updated, ctid is the location
* of the replacement tuple and update_xmax is the updating
transaction's xid.
* update_xmax must in this case be used to verify that the replacement
tuple
* matched.
* Otherwise, if ctid is the same as t_self and update_xmax is
* InvalidTransactionId, the tuple was neither replaced nor deleted, but
* locked by a transaction invisible to lockcheck_snapshot. This case can
* thus only arise if lockcheck_snapshot is a valid snapshot.
That's quite complicated. I think we should bite the bullet and add a
couple of more return codes to explicitly tell the caller what happened.
I propose:
HeapTupleMayBeUpdated- the tuple was actually updated (same as before)
HeapTupleSelfUpdated - the tuple was updated by a later command in same
xact (same as before)
HeapTupleBeingUpdated - concurrent update in progress (same as before)
HeapTupleUpdated - the tuple was updated by another xact. *update_xmax
and *ctid are set to point to the replacement tuple.
HeapTupleDeleted - the tuple was deleted by another xact
HeapTupleLocked - lockcheck_snapshot was given, and the tuple was locked
by another xact
I'm not sure how to incorporate that into the current
heap_delete/update/lock_tuple functions and HeapTupleSatisfiesUpdate. It
would be nice to not copy-paste the logic to handle those into all three
functions. Perhaps that common logic starting with the
HeapTupleSatisfiesUpdate() call could be pulled into a common function.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
