List:Falcon Storage Engine« Previous MessageNext Message »
From:Olav Sandstaa Date:November 24 2008 9:50pm
Subject:Re: Question about Transaction::getRelativeState called with null
View as plain text  
Hi Kevin,

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?


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

Question about Transaction::getRelativeState called with null valuesOlav Sandstaa24 Nov
  • Re: Question about Transaction::getRelativeState called with nullvaluesKevin Lewis24 Nov
    • Re: Question about Transaction::getRelativeState called with nullvaluesOlav Sandstaa24 Nov
      • Re: Question about Transaction::getRelativeState called with nullvaluesKevin Lewis24 Nov