Thanks again for taking the time to comment and provide suggestions, I
really appreciate feedback.
See replies further down:
Kevin Lewis wrote:
> >> Do you think it is OK to remove the call to
> >> Transaction::commitRecords() from Transaction::writeComplete()?
> Yes, I think that would be OK.
>>> Yes, I agree that this code will have to be extended if/when there
>>> can be "available" transaction objects in the (beginning of) the
>>> list. The problem with this is that it increases the cost of finding
>>> the "oldest active". Jim's original proposal was to have the active
>>> list sorted with the oldest active at the front of the list.
>>> Today I have implemented a patch for what I suggested in the email I
>>> sent yesterday (as described above). Basically, I have moved the
>>> call to TransactionManager::purgeTransactions() from being called by
>>> the scavenger to being called by Transaction::commit(). The two main
>>> reasons for this. The first is to call Transaction::commitRecords()
>>> and delete old transaction commits much earlier than when using the
>>> scavenger for this. The second reason is that in
>>> Transaction::commit() we anyway have the necessary locks set on the
>>> active and committed transaction lists. The cost of this strategy
>>> will be slightly more costly than having this done as a "batch" by
>>> the scavenger but we will free up memory used for records and
>>> transaction objects much earlier. And if we have to do a search for
>>> the oldest active in the active list this will further increase the
>>> cost slightly.
>> I just committed this patch. It moves the call to
>> TransactionManager::purgeTransactions() from being called by the
>> scavenger to being called by Transaction::commit(). So instead of the
>> scavenger doing "batch" processing of this job it is now done on
>> every commit (soon: only by non-read-only transactions).
> When TransactionManager::purgeTransactions() is called only part of
> the time in Transaction::commit(), We may need to keep calling
> TransactionManager::purgeTransactions() in the scavenger as a backup.
> Imagine a phase of really high load of selects with no writes
> happening. It would be good for the scavenger to finish up purging
> whatever transactions did not get purged by the last update
> transaction commit.
> I don't think it hurts to have the scavenger call
I agree with you that it does not hurt to have the scavenger call
TransactionManager::purgeTransactions and that this can in some cases as
the example you mentioned clean up some transactions that otherwise
would hang around for a long time.
I will update the patch to let the scavenger still call
Transaction::purgeTransactions so that it implements 1 and 3 (see below).
>> Still, I am open for input on what people think is the best solution:
>>> I think the main alternatives for calling
>>> Transaction::commitRecords() and deleting old transactions objects are:
>>> 1. Transaction::commit() calls TransactionManager::purgeTransactions()
>>> 2. Transaction::writeComplete calls Transaction::commitRecords()
>>> 3. The scavenger calls TransactionManager::purgeTransactions()
> For reasons above, I vote 1 AND 3
>> A "bonus advantage" of this approach is that this is done only by a
>> single code path, not five different code paths as with the old
>> dependency manager.
>> I have done a simple performance test and do not see more than what
>> can be expected of measurement variation between the approach of
>> batchprocessing up to 300.000 transaction objects by the scavenger
>> compared to do it this in every commit. The the two lower lines shows
>> the performance of alternative 1 and 3 above for 1 to 50 clients
>> doing a simple select of a "random record" from a table.