Re: New Transaction State object (Was: Problems with record visibility and how it is computed)

[Olav Sandstaa]

Hi Jim,

Thanks again for your comments. I really appreciate that you look at 
what I am working on.

See a response further down:

Jim Starkey wrote:
> Olav Sandstaa wrote:
>
>>
>>>    * syncPending (so it can be waited on)
>>
>> The main problem with including this is that it increases the size of 
>> the TransactionState object from about 12 bytes to about 150 bytes 
>> (at least with the currently enabled tracing code). How many of these 
>> can "extreme testcases" produce?
>>
>> In the initial prototype I am working on I have moved the 
>> syncIsActive (which is hopefully the same as you refer to as 
>> syncPending) to the TransactionState object.
>> We are only waiting for active transactions so in all committed 
>> TransactionState object this syncObject will never be used. In the 
>> current implementation of Transaction::waitForTransaction() already 
>> has a shared lock on the active transaction list. So I thing it will 
>> be possible to ensure that we safely can use the syncObject also when 
>> it is part of the Transaction object. The advantage of this is that 
>> we do not need to ten-double the size of the TransactionState object.
>>
>> The drawback is that we can not easily get rid of neither the current 
>> shared lock on the active transaction list or the active transaction 
>> list (which I see you are aiming for). (and yes, I have an idea for 
>> how to get rid of the currently used shared lock in 
>> waitForTransaction()).
> The question is how one transaction waits for another.  Does this work 
> (or is there a simpler way):
>
>   1. Get a shared lock on active transaction list
>   2. Call addRef on transaction to wait for
>   3. Release shared lock on active transaction list
>   4. Wait on other transaction syncPending
>   5. Release syncPending
>   6. Call release on transaction waited on

This is about how Transaction::waitForTransaction() is implemented 
today. But I think there is one step missing (and that is also the cause 
for one of the crashes seen a few times: bug#41665). (in addition step 1 
and 2 in the current implementation is in reverse order - which is not 
good either).

The problem with the above is step 2 (I think): Even if you get a shared 
lock on the active transaction list there is no guarantee that the 
transaction still is in the active transaction list when you call 
addRef() on it (and might be deleted). To get around this, I see two ways:

 1b: search for the transaction in the active list - if found, then 
continue, if not found then assume it is committed (and return) (and NO 
I do not like searching the list)
 1b: check the transaction state object: if this still says that the 
state of the transaction is "Active or Limbo" then you can do the 
addRef() on the transaction object.

(but in the working version I have right now the 
syncIsActive/syncPending object is moved to the Transaction State object 
and we do not have this issue - (just a "fat" transaction state object :-( )

Olav


>
>>
>>>
>>> This makes TransactionState the primary operational object and 
>>> Transaction mostly for bookkeeping.  I think this also eliminates 
>>> the need for TransactionManager::activeTransactions.
>>
>> I agree but that should likely be a next step goal and not part of 
>> this (which started as a bug fix for 41357).
> It occurred to me that we need the active transaction list to track 
> down transaction that have updates to tables that the MySQL server 
> wants to delete.  I don't see anyway around it, at least not for now.  
> I suggest we put this dream aside until less strenuous times.
>
>
>