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.
* 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)