From: Kevin Lewis
Date: November 24 2008 10:15pm
Subject: Re: Question about Transaction::getRelativeState called with null
 values
List-Archive: http://lists.mysql.com/falcon/210
Message-Id: <492B2797.9090805@sun.com>
MIME-Version: 1.0
Content-Type: text/plain; format=flowed; charset=ISO-8859-1
Content-Transfer-Encoding: 7BIT

 > 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
>>>
>>>
>>>
> 
>