From: Kevin Lewis
Date: December 4 2008 6:06pm
Subject: Re: Dependency manager and Transaction::commitRecords()
List-Archive: http://lists.mysql.com/falcon/259
Message-Id: <49381C09.6000104@sun.com>
MIME-Version: 1.0
Content-Type: text/plain; format=flowed; charset=ISO-8859-1
Content-Transfer-Encoding: 7BIT

Olav,  Important point below...


Olav Sandstaa wrote:
> Hi,
> 
> Just sharing some information if someone is interested or feel for 
> commenting - feedback is appreciated (but feel free to press the delete 
> (or power)  button too):
> 
> One of the uses of the old (as well as the new) dependency manager is to 
> determine when it is safe to call Transaction::commitRecords() (and in 
> most cases also being able to delete/release the transaction object 
> itself). When using the old dependency manager one of the requirement 
> was that the dependency counter had reached zero before calling 
> Transaction::commitRecords().
> 
> In the code using the old dependency manager 
> Transaction::commitRecords() could be called in four/five different places:
> 
> 1. Transaction::commit()
> 
> 2 a+b). Transaction::releaseDependency() called either from 
> Transaction::expungeTransaction() or Transaction::releaseDependencies().
> 
> 3. Transaction::writeComplete() (callback from serial log/gopher thread)
> 
> 4. TransactionManager::purgeTransaction() (clean-up of transaction by 
> the scavenger).
> 
> In the initial patch for the new dependency manager 
> Transaction::commitRecords is called only from one place: 
> Transaction::purgeTransaction() (case 4 above), i.e. it is not called 
> until the Scavenger comes around (about every 30 seconds) [1]. Case 2a+b 
> has been eliminated by removing the old dependency manager code. The 
> code for case 1 and 3 have been temporarily commented out with a comment 
> that "this code will be fixed by the next patch" :-) . Still, with only 
> case 4 in the code it seems that Falcon works both correctly and "nice" 
> but it might not be optimal to delay all calls to 
> Transaction::commitRecords() until the scavenger comes around.
> 
> So to get some data about which of the above cases that actually are in 
> use I instrumented the code and ran the falcon test suite. The result is 
> (number of calls to commitRecords()):
> 
> Transaction::commit:                                      1074
> Transaction::releaseDependency:                10839
> Transaction::writeComplete:                      179389
> TransactionManager::purgeTransaction: 168713
> 
> Some more instrumentation revealed that for most Transactions 
> commitRecords() was called twice. And further, in the table above *all* 
> calls done by Transaction::commit() and 
> TransactionManager::purgeTransactions() actually was a "second" call to 
> Transaction::commitRecords().
> 
> This leaves me believe/suggest the following for the new dependency 
> manager for how and when to call commitRecords() (and delete committed 
> transaction objects):
> 
> * Case 1 above (in Transaction::commit()) is not necessary at all - 
> remove it.
> 
> * Case 2a+b: is already gone since the old dependency manger code is gone
> 
> * Case 3: is very useful but in order to re-implement the "old 
> dependency check" that just checks that the dependencies counter is 
> zero, with the new dependency manager I would need to introduce a shared 
> lock on the active transaction list to get the "startEvent"  (also now 
> known as the transactionId) of the oldest active transaction in order to 
> do the "new dependency check". This does not seem like the best thing to 
> do.
> 
>  So instead (based on a suggestion from Jim) I think the best (and 
> easiest) way to call Transaction::commitRecords() and delete old 
> transaction objects will be to do something similar to what the new 
> implementation of TransactionManager::purgeTransaction() does each time 
> we commit a record when we anyway have the exclusive lock on both the 
> active and committed lists. This code will call commitRecords() and 
> release the transaction objects from the front of the committed 
> transaction list as long as they  have a commitId (commitEvent) that is 
> smaller than the transactionId (startEvent) of the oldest active 
> transactions. The cost per transaction object should be very "similar" 
> to the cost of doing the same in the scavenger.

You need to get at least a shared lock on 
TransactionManager::activeTransactions to find the oldest active 
transaction.

In your code you do this in purgeTransactions();

	oldestActive = activeTransactions.first->startEvent;

The old code used a shared lock in 
TransactionManager::findOldestActive() which called 
TransactionManager::findOldest().

You may need to use TransactionManager::findOldest() which does get that 
shared lock.  It scans the whole list because is assumes that there are 
some transactions in that list that have status=available.  Your current 
patch does not have and 'available' transactions in the active list, but 
it will when you add back in commitNoUpdates and allocations of 10 at a 
time.

Kevin


> 
> * Case 4: TransactionManager::purgeTransaction() can probably be deleted 
> (if I implement what suggested above).
> 
> One extra benefit of this that instead of having five code paths that 
> lead to commitRecords() in the old implementation, the new 
> implementation will be simpler with commitRecords() only called in a 
> single place.
> 
> Everyone that is still reading and think they understand what I wrote: 
> feel free to comment as I might have misunderstood or jumped to wrong 
> conclusions. Everyone else: feel free to ask questions :-)
> 
> Thanks for listening! (and yes, I will update the worklog....)
> 
> Olav
> 
> [1] The "record number" of committed transaction ready for clean-up by 
> the scavenger I have seen in one of my tests is 330.000 transactions :-) 
> (but that will not be typical because in the initial patch also 
> read-only transactions ends up in the committed list)
> 
> 
>