> Olav Sandstaa wrote:
> Thanks for the explanation and confirming my observation. I agree with
> your suggestion and think the code looks better and is more robust when
> testing explicitly for the situation where we have a record object and
> no associated transaction.
>
> I have run some tests with your suggested change and it seems to work
> very well. Do you want to check in this change yourself or should I just
> include this in one of my patches?
Please include this in one of your patches.
>
> Olav
>
>
> Kevin Lewis wrote:
>> Olav,
>>
>> I'm glad you noticed this. The relative state of a Record object is
>> always CommittedVisible. It was never associated wioth a transaction.
>> Just a committed record read from the database. We might as well save
>> time and report that directly. Try this;
>>
>> State Transaction::getRelativeState(Record* record, uint32 flags)
>> {
>> + if (!record->isVersion())
>> + return CommittedVisible;
>> +
>> blockingRecord = record;
>> State state = getRelativeState(record->getTransaction(),
>> record->getTransactionId(), flags);
>> blockingRecord = NULL;
>>
>> return state;
>> }
>>
>>
>> Olav Sandstaa wrote:
>>> 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
>>>
>>>
>>>
>
>
