Summary of the summary discussion, with a few new notes from
>>>> 1. We can stop using the pointer to the transaction and instead
>>>> locate the transaction by searching for it
>>> TransactionManager::findTransaction does that, [but]
>>> it requires a shared lock and release.
>>> That's way too expensive for checking record visibility.
>>>> 2. We can acquire the shared lock on the committed transaction
>>>> list each time we call Transaction::getRelativeState and
>> Too much contention on the committed transaction list.
>>>> 3. Introduce a new shared lock for just this purpose to
>>>> avoid the contention on the committed transaction list
>>>> lock. When purging transactions we acquire an exclusive lock on
>>>> this SyncObject
>>> We need to avoid both locks and interlocked instructions
>>> (increments and decrements) [for the purpose of checking
>>> record visibility] if we're going to keep performance up.
>> The Transaction class has a useCount. Call Transaction::addRef()
>> for every RecordVersion added to the transaction in
>> Transactino::addRecord(). Then also for every stack based use of that
>> pointer, call addref and release.
> There isn't much difference in cost between a lock(Shared)/unlock and
> addRef/release. Each requires a pair of interlocked instructions.
>>>> 4. Duplicate the commit information (one integer) in every
>>>> RecordVersion object. When committing a transaction,
>>>> update every record version with the commitId of the transaction
>> This needs to happen while both transaction lists are locked because
>> that is where the commitId is assigned to the transaction.
OK, you're a transaction that's committing. You take yourself off
the active list and put yourself onto the committed list. Until you
get all your records marked, you don't want anybody else to start -
they'd see your transaction as committed but might not see all your
records as being in that state.
>>>> 5. Never deleting the transaction object until all record
>>>> objects pointing to it have been deleted. This would also simplify
>>>> some of the code since we would always have the transaction
>>> That's a lot of dead transaction objects. And it isn't just records
>>> that point to the transaction but also threads ...
>> Records would be detached from these transactions by the scavenger.
>> Records will always have that transaction pointer.
Could we make the transaction pointer indirect? It points to a
TransactionState object that contains the commitID if there is
one, and the address of the Transaction if there isn't a commitID.
That at least reduces the amount of dreck we carry around for
long periods of time.
>> Now, will the committed transaction list get too long? Maybe.
>> But this might also allow us to retire records sooner. The current
>> scavenger cannot retire Records until after they have been separated
>> from their transaction (purgeTransactions). But this would allow the
>> scavenger to do the separating, and retire records that are just not
>> needed by any other active transaction. So this solution has the
>> benefit of allowing records to be retired sooner. See the email chain
>> titled "Reluctant Scavenger" for a possible example of this problem.
I'm not convinced that we can guarantee that record versions go away.
People show up on the Firebird lists regularly with amazingly bloated
databases and a transaction or two that have been active for a week.
To handle that case, we'd need to identify unneeded intermediate
transactions and remove them ....
>>>> 6. Cycle locking
>>> I've got cycle locking on the brain.
>> But this change is a big deal.
>> 7. Yes, I have another alternative. Probably the easiest of all.
>> Comment out this line;
>> This is new with Olav's dependency manager change and it causes
>> transactions to be purged much sooner than a scavenger cycle. It
>> extends the CPU cycles during the critical/serialized part of every
>> commit and may actually be to blame for a recent performance slowdown
>> in DBT2. (that is a big maybe! Not sure about that at all). But if we
>> leave the transactions around until the scavenge cycle, this error
>> might not happen any more...
> Doesn't this leave the race between picking up a transaction pointer
> from a record and deleting the transaction object persist?
> You need to demonstrate it works in all circumstances.
> The theoretical problem is that there generally accessible pointers to
> transaction objects, so deleting the transaction object is problematic.
> I think the only solutions involve:
> * Eliminating the pointer to the transaction
> * A reference count of the transaction object held by the record object
> * A lock on the transaction object held by the record object
> * A lock on transaction existence for the duration of record
> visibility cycle
Which I think actually is two cases:
1) Don't have record versions point to Transactions or
2) Make Transactions persist at least as long as their record versions
And at the same time, be fast, highly concurrent, and recycle old bits.