Hi Kevin,
After applying your patch for Bug #40893 "Falcon concurrent unlock can
cause invalid fetches" I found another issue with the use of
Transaction::getRelativeState() that I first thought had to be wrong but
which might be correct anyway. It would be good to get your comment on it.
It is related to the following code:
State Transaction::getRelativeState(Transaction *transaction, TransId
transId, uint32 flags)
{
if (transactionId == transId)
return Us;
// A record may still have the transId even after the trans itself
has been deleted.
if (!transaction)
{
// All calls to getRelativeState are for the purpose of writing.
// So only ConsistentRead can get CommittedInvisible.
if (IS_CONSISTENT_READ(isolationLevel))
{
// If the transaction is no longer around, and the record is,
// then it must be committed.
if (transactionId < transId)
return CommittedInvisible;
// Be sure it was not active when we started.
for (int n = 0; n < numberStates; ++n)
if (states [n].transactionId == transId)
return CommittedInvisible;
}
return CommittedVisible;
}
After your fix I had hoped that we no longer should see cases where we
use the states array in the last for-loop. But with the debug code I had
in my code tree we were still executing that code (but not the return
statement). So I wondered why....
It turns out that the case where this code is executed happens when we
call Transaction::getRelativeState(Transaction*...) and the transaction
argument is NULL and the transId is 0. This seems to happen when we
calls Transaction::getReleativeState(Record* ) and the record argument
is a Record object, not a RecordVersion object.
When this is the case, Transaction::getRelativeState() will always
return CommittedVisible (which is likely the correct value). Still from
reading the above code it is a bit hard to imagine that it is supposed
to work when both transaction==NULL and transId==0 (and could easily be
a source for introducing unintended changes if someone decided to change
the order of the logic).
At least this puzzled me, so is it correct use of
Transaction::getTransaction(NULL, 0, flags) in this case?
Olav
