Heikki Linnakangas <heikki.linnakan...@enterprisedb.com> wrote:
> On 30.05.2011 17:10, Kevin Grittner wrote:
>> Heikki Linnakangas wrote:
>>> * XXX: for an anomaly to occur, T2 must've committed
>>> * before W. Couldn't we easily check that here, or does
>>> * the fact that W committed already somehow imply it?
>>
>> The flags and macros should probably be renamed to make it more
>> obvious that this is covered. The flag which SxactHasConflictOut
>> is based on is only set when the conflict out is to a transaction
>> which committed ahead of the flagged transaction. Regarding the
>> other test -- since we have two transactions (Tin and Tout) which
>> have not been summarized, and transactions are summarized in
>> order of commit, SxactHasSummaryConflictOut implies that the
>> transaction with the flag set has not committed before the
>> transaction to which it has a rw-conflict out.
>
> Ah, got it.
>
>> The problem is coming up with a more accurate name which isn't
>> really long. The best I can think of is
>> SxactHasConflictOutToEarlierCommit, which is a little long. If
>> nobody has a better idea, I guess it's better to have a long name
>> than a misleading one. Do you think we need to rename
>> SxactHasSummaryConflictOut or just add a comment on that use that
>> a summarized transaction will always be committed ahead of any
>> transactions which are not summarized?
>
> Dunno. I guess a comment would do. Can you write a separate patch
> for that, please?
Attached is a comments-only patch for this, along with one
correction to the comments you added and a couple other typos.
I'll submit a patch for the README-SSI file once I find a reference
I like to a paper describing what Dan's proof uses as a premise --
that the transaction on the rw-conflict *out* side of the pivot must
not only be the first of the three transactions in the dangerous
structure to commit, but the first in the entire cycle of which the
dangerous structure is a part. With that premise as a given, the
proof is short, clear, and unassailable; but I think we need a cite
to use that argument convincingly.
-Kevin
*** a/src/backend/storage/lmgr/predicate.c
--- b/src/backend/storage/lmgr/predicate.c
***************
*** 245,250 ****
--- 245,255 ----
#define SxactIsReadOnly(sxact) (((sxact)->flags & SXACT_FLAG_READ_ONLY) != 0)
#define SxactHasSummaryConflictIn(sxact) (((sxact)->flags &
SXACT_FLAG_SUMMARY_CONFLICT_IN) != 0)
#define SxactHasSummaryConflictOut(sxact) (((sxact)->flags &
SXACT_FLAG_SUMMARY_CONFLICT_OUT) != 0)
+ /*
+ * The following macro actually means that the specified transaction has a
+ * conflict out *to a transaction which committed ahead of it*. It's hard
+ * to get that into a name of a reasonable length.
+ */
#define SxactHasConflictOut(sxact) (((sxact)->flags &
SXACT_FLAG_CONFLICT_OUT) != 0)
#define SxactIsDeferrableWaiting(sxact) (((sxact)->flags &
SXACT_FLAG_DEFERRABLE_WAITING) != 0)
#define SxactIsROSafe(sxact) (((sxact)->flags & SXACT_FLAG_RO_SAFE) != 0)
***************
*** 2708,2714 **** SetNewSxactGlobalXmin(void)
* up in some relatively timely fashion.
*
* If this transaction is committing and is holding any predicate locks,
! * it must be added to a list of completed serializable transaction still
* holding locks.
*/
void
--- 2713,2719 ----
* up in some relatively timely fashion.
*
* If this transaction is committing and is holding any predicate locks,
! * it must be added to a list of completed serializable transactions still
* holding locks.
*/
void
***************
*** 2753,2764 **** ReleasePredicateLocks(const bool isCommit)
LWLockAcquire(SerializableXactHashLock, LW_EXCLUSIVE);
/*
! * We don't hold a lock here, assuming that TransactionId is atomic!
*
* If this value is changing, we don't care that much whether we get the
* old or new value -- it is just used to determine how far
! * GlobalSerizableXmin must advance before this transaction can be
cleaned
! * fully cleaned up. The worst that could happen is we wait for ome
more
* transaction to complete before freeing some RAM; correctness of
visible
* behavior is not affected.
*/
--- 2758,2770 ----
LWLockAcquire(SerializableXactHashLock, LW_EXCLUSIVE);
/*
! * We don't hold an XidGenLock lock here, assuming that TransactionId is
! * atomic!
*
* If this value is changing, we don't care that much whether we get the
* old or new value -- it is just used to determine how far
! * GlobalSerizableXmin must advance before this transaction can be fully
! * cleaned up. The worst that could happen is we wait for one more
* transaction to complete before freeing some RAM; correctness of
visible
* behavior is not affected.
*/
***************
*** 3860,3867 **** OnConflict_CheckForSerializationFailure(const
SERIALIZABLEXACT *reader,
* Because T2 must've committed first, there is no anomaly if:
* - the reader committed before T2
* - the writer committed before T2
! * - the reader is a READ ONLY transaction and the reader was not
! * concurrent with T2 (= reader acquired its snapshot after T2
committed)
*/
if (!failure)
{
--- 3866,3873 ----
* Because T2 must've committed first, there is no anomaly if:
* - the reader committed before T2
* - the writer committed before T2
! * - the reader is a READ ONLY transaction and the reader was concurrent
! * with T2 (= reader acquired its snapshot before T2 committed)
*/
if (!failure)
{
*** a/src/include/storage/predicate_internals.h
--- b/src/include/storage/predicate_internals.h
***************
*** 92,97 **** typedef struct SERIALIZABLEXACT
--- 92,102 ----
#define SXACT_FLAG_ROLLED_BACK 0x00000001
#define SXACT_FLAG_COMMITTED 0x00000002
+ /*
+ * The following flag actually means that the flagged transaction has a
+ * conflict out *to a transaction which committed ahead of it*. It's hard
+ * to get that into a name of a reasonable length.
+ */
#define SXACT_FLAG_CONFLICT_OUT 0x00000004
#define SXACT_FLAG_READ_ONLY 0x00000008
#define SXACT_FLAG_DID_WRITE 0x00000010
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
