On 15.01.2011 01:54, Kevin Grittner wrote:
/*
* for r/o transactions: list of concurrent r/w transactions that we
could
* potentially have conflicts with, and vice versa for r/w transactions
*/
TransactionId topXid; /* top level xid for the transaction,
if one
* exists; else
invalid */
TransactionId finishedBefore; /* invalid means still running;
else
* the struct expires when no
* serializable xids are before this. */
TransactionId xmin; /* the transaction's snapshot
xmin */
uint32 flags; /* OR'd combination of values
defined below */
int pid; /* pid of associated
process */
} SERIALIZABLEXACT;
What does that comment about list of concurrent r/w transactions refer
to? I don't see any list there. Does it refer to
possibleUnsafeConflicts, which is above that comment?
Above SERIALIZABLEXID struct:
* A hash table of these objects is maintained in shared memory to provide a
* quick way to find the top level transaction information for a serializable
* transaction, Because a serializable transaction can acquire a snapshot
* and read information which requires a predicate lock before it has a
* TransactionId, it must be keyed by VirtualTransactionId; this hashmap
* allows a fast link from MVCC transaction IDs to the related serializable
* transaction hash table entry.
I believe the comment is trying to say that there's some *other* hash
that is keyed by VirtualTransactionId, so we need this other one keyed
by TransactionId. It took me a while to understand that, it should be
rephrased.
Setting the high bit in OldSetXidAdd() seems a bit weird. How about just
using UINT64_MAX instead of 0 to mean no conflicts? Or 1, and start the
sequence counter from 2.
ReleasePredicateLocks() reads ShmemVariableCache->nextXid without
XidGenLock. Maybe it's safe, we assume that TransactionIds are atomic
elsewhere, but at least there needs to be a comment explaining it. But
it probably should use ReadNewTransactionId() instead.
Attached is a patch for some trivial changes, mostly typos.
Overall, this is very neat and well-commented code. All the data
structures make my head spin, but I don't see anything unnecessary or
have any suggestions for simplification. There's a few remaining TODO
comments in the code, which obviously need to be resolved one way or
another, but as soon as you're finished with any outstanding issues that
you know of, I think this is ready for commit.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
diff --git a/doc/src/sgml/mvcc.sgml b/doc/src/sgml/mvcc.sgml
index 2024b48..c8078fe 100644
--- a/doc/src/sgml/mvcc.sgml
+++ b/doc/src/sgml/mvcc.sgml
@@ -29,7 +29,7 @@
</indexterm>
<indexterm>
- <primary>Serializable Snapshot Isloation</primary>
+ <primary>Serializable Snapshot Isolation</primary>
</indexterm>
<indexterm>
@@ -61,9 +61,9 @@
do not conflict with locks acquired for writing data, and so
reading never blocks writing and writing never blocks reading.
<productname>PostgreSQL</productname> maintains this guarantee
- even when providing the the strictest level of transaction
+ even when providing the strictest level of transaction
isolation through the use of an innovative <firstterm>Serializable
- Snapshot Isloation</firstterm> (<acronym>SSI</acronym>) level.
+ Snapshot Isolation</firstterm> (<acronym>SSI</acronym>) level.
</para>
<para>
@@ -647,7 +647,7 @@ ERROR: could not serialize access due to read/write dependencies among transact
</listitem>
<listitem>
<para>
- Don't leave connections dangling "idle in transction" longer than
+ Don't leave connections dangling "idle in transaction" longer than
necessary.
</para>
</listitem>
diff --git a/src/backend/storage/lmgr/README-SSI b/src/backend/storage/lmgr/README-SSI
index 7eb82be..2c4de29 100644
--- a/src/backend/storage/lmgr/README-SSI
+++ b/src/backend/storage/lmgr/README-SSI
@@ -207,7 +207,7 @@ until one of the other transactions in the set of conflicts which
could generate an anomaly has successfully committed. This is
conceptually similar to how write conflicts are handled. To fully
implement this guarantee there needs to be a way to roll back the
-active transaciton for another process with a serialization failure
+active transaction for another process with a serialization failure
SQLSTATE, even if it is "idle in transaction".
@@ -286,7 +286,7 @@ index which would have been included in the scan had they existed at
the time. Conceptually, we want to lock the gaps between and
surrounding index entries within the scanned range.
-Correctness requires that any insert into an index generate a
+Correctness requires that any insert into an index generates a
rw-conflict with a concurrent serializable transaction if, after that
insert, re-execution of any index scan of the other transaction would
access the heap for a row not accessed during the previous execution.
@@ -404,7 +404,7 @@ to balance RAM usage against SIREAD lock granularity.
covering locks with counts, are kept to support granularity promotion
decisions with low CPU and locking overhead.
- * Conflicts will be identified at by looking for predicate locks
+ * Conflicts will be identified by looking for predicate locks
when tuples are written and looking at the MVCC information when
tuples are read. There is no matching between two RAM-based locks.
@@ -445,7 +445,7 @@ overlapping transaction dependencies.
* Allow the user to request that a READ ONLY transaction wait
until the conditions are right for it to start in the "opt out" state
described above. We add a DEFERRABLE state to transactions, which is
-specified and maintained in a way similar to to READ ONLY. It is
+specified and maintained in a way similar to READ ONLY. It is
ignored for transactions which are not SERIALIZABLE and READ ONLY.
* When a transaction must be rolled back, we pick among the
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index 4f718f3..71557ef 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -770,7 +770,7 @@ OldSerXidAdd(TransactionId xid, SerCommitSeqNo minConflictCommitSeqNo)
LWLockRelease(OldSerXidLock);
- return false; /* TODO SSI: How to deterimine wraparound
+ return false; /* TODO SSI: How to determine wraparound
* close? */
}
@@ -1271,8 +1271,7 @@ SummarizeOldestCommittedSxact(void)
LWLockRelease(SerializableFinishedListLock);
/* Add to SLRU summary information. */
- if (TransactionIdIsValid(sxact->topXid)
- && !SxactIsReadOnly(sxact))
+ if (TransactionIdIsValid(sxact->topXid) && !SxactIsReadOnly(sxact))
OldSerXidAdd(sxact->topXid, SxactHasConflictOut(sxact)
? sxact->SeqNo.earliestOutConflictCommit : (SerCommitSeqNo) 0);
@@ -2076,16 +2075,18 @@ PredicateLockTuple(const Relation relation, const HeapTuple tuple)
*/
if (relation->rd_index == NULL)
{
- TransactionId xid;
-
- xid = HeapTupleHeaderGetXmin(tuple->t_data);
- if (TransactionIdFollowsOrEquals(xid, TransactionXmin))
+ TransactionId myxid = GetToptransactionIdIfAny();
+ if (TransactionIdIsValid(myxid))
{
- xid = SubTransGetTopmostTransaction(xid);
- if (xid == GetTopTransactionIdIfAny())
+ TransactionId xid = HeapTupleHeaderGetXmin(tuple->t_data);
+ if (TransactionIdFollowsOrEquals(xid, TransactionXmin))
{
- /* We wrote it; we already have a write lock. */
- return;
+ xid = SubTransGetTopmostTransaction(xid);
+ if (TransactionIdEquals(xid, myxid))
+ {
+ /* We wrote it; we already have a write lock. */
+ return;
+ }
}
}
}
@@ -2562,6 +2563,10 @@ ReleasePredicateLocks(const bool isCommit)
LWLockAcquire(SerializableXactHashLock, LW_EXCLUSIVE);
+ /*
+ * XXX: We don't hold a lock here, assuming that TransactionId is atomic!
+ * should this at least be volatile? What happens if we get this wrong?
+ */
MySerializableXact->finishedBefore = ShmemVariableCache->nextXid;
/*
@@ -2673,7 +2678,7 @@ ReleasePredicateLocks(const bool isCommit)
}
/*
- * Release all inConflicts from committed and read-only transactions. If
+ * Release all inConflicts from committed and read-only transactions. If
* we're rolling back, clear them all.
*/
conflict = (RWConflict)
@@ -2936,7 +2941,7 @@ ClearOldPredicateLocks(void)
* delete the transaction.
*
* When the partial flag is set, we can release all predicate locks and
- * out-conflict information -- we've esablished that there are no longer
+ * out-conflict information -- we've established that there are no longer
* any overlapping read write transactions for which this transaction could
* matter.
*
diff --git a/src/include/storage/predicate_internals.h b/src/include/storage/predicate_internals.h
index 6d1fcb4..2f5f8d4 100644
--- a/src/include/storage/predicate_internals.h
+++ b/src/include/storage/predicate_internals.h
@@ -34,7 +34,7 @@ typedef struct SERIALIZABLEXACTTAG
} SERIALIZABLEXACTTAG;
/*
- * The SERIALIZABLEXACT struct conatins information needed for each
+ * The SERIALIZABLEXACT struct contains information needed for each
* serializable database transaction to support SSI techniques.
*
* A hash table is maintained in shared memory of these, keyed by the virtual
@@ -137,7 +137,7 @@ typedef struct PredTranListData
* transactions are active */
SerCommitSeqNo LastSxactCommitSeqNo; /* a strictly monotonically
* increasing number for
- * commits of serialiable
+ * commits of serializable
* transactions */
SerCommitSeqNo LastWritingCommitSeqNo; /* The last commitSeqNo
* assigned at commit to a
@@ -208,7 +208,7 @@ typedef struct SERIALIZABLEXIDTAG
*
* A hash table of these objects is maintained in shared memory to provide a
* quick way to find the top level transaction information for a serializable
- * transaction, Because a serializable transaction can acquire a snapshot
+ * transaction. Because a serializable transaction can acquire a snapshot
* and read information which requires a predicate lock before it has a
* TransactionId, it must be keyed by VirtualTransactionId; this hashmap
* allows a fast link from MVCC transaction IDs to the related serializable
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
