Kevin Lewis wrote:
> Comments below;
>
> Olav Sandstaa wrote:
>>
>> Jim and everybody else,
>>
>> As I mentioned in was working on removing the last parts of the old
>> dependency manager and had one issue left (famous last word :-) ).
>> This issue is related to Transaction::getRelativeState().
>>
>> Some background: The new dependency manager stores the event counter
>> for (a) when the transaction became active and (b) when the
>> transaction committed in the transaction object. These two event
>> numbers is then used for comparing the relative state of active and
>> committed transactions. Using these instead of the old dependency
>> manager allows me to remove (a) the dependency counter for each
>> transaction and (b) the states array with pointers to all transactions
>> that where active at the start of a given transaction.
>>
>> The issue:
>>
>> The code for the first part of Transaction::getRelativeState() looks
>> like this:
>>
>>
>> State Transaction::getRelativeState(Transaction *transaction, TransId
>> transId, uint32 flags)
>> {
>> if (transactionId == transId)
>> return Us;
>>
>> // A record may still have the transId even after the trans itself
>> has been deleted.
>> if (!transaction)
>> {
>> // All calls to getRelativeState are for the purpose of writing.
>> // So only ConsistentRead can get CommittedInvisible.
>>
>> if (IS_CONSISTENT_READ(isolationLevel))
>> {
>> // If the transaction is no longer around, and the record is,
>> // then it must be committed.
>>
>> if (transactionId < transId)
>> return CommittedInvisible;
>>
>> // Be sure it was not active when we started.
>>
>> for (int n = 0; n < numberStates; ++n)
>> if (states [n].transactionId == transId)
>> return CommittedInvisible;
>> }
>>
>> return CommittedVisible;
>> }
>>
>>
>> The issue is related to when the pointer to the transaction argument
>> is NULL:
>>
>> 1) Not having a reference to the transaction makes it hard(er) to
>> compare the state of two different transactions since the one we are
>> going to compare state against is "not there". For the old dependency
>> manager this was easy since it has the states array of all active
>> transactions available also after these transactions possibly are
>> deleted (see the for loop above). But with the new dependency manager
>> we should be able to remove this state array....
>>
>> I have a working solution for this: By including the commit
>> transition event number in every RecordVersion object (I set this in
>> Transaction::commit()) makes it possible to compare this record
>> against this transaction's start event. The code replacing the above
>> for loop looks approximately like this:
>>
>> if (transEndEvent_of_recordObject > startEvent)
>> {
>> fprintf(stderr, "T::getRelativeState: %d: NEW:
>> CommittedInvisible: end=%d start=%d\n", transactionId, transEndEvent,
>> startEvent);
>> return CommittedInvisible;
>> }
>>
>> The test for this is simple but I do not like to having to include
>> the commit event in record versions if I do not have to do so. Anyone
>> having a suggestion for a better solution?
>
> This all hinges on the design that the transaction is NULLed out during
> Transaction::commitRecords(). This can happen at Transaction::commit()
> or Transaction::writeComplete() or even later at
> Transaction::releaseDependency(). But it always requires that
> (dependencies == 0) && !writePending)
>
> In fact, it is effectively impossible for the
> Transaction::commitRecords() to be called in Transaction::commit() since
> it requires !writePending. I think that call is vestigial from the days
> when commitNoUpdates() did not exist and some of the transaction going
> through Transaction::commit() were read-only.
>
> How are you managing the dependency counter? Without it, you may be
> separating the transaction from its records too soon. Before, if the
> dependency counter went to zero, then this transaction ended before any
> active currently transaction. There would be no overlap because the
> overlap was managed by the dependency counter and the states array.
>
> In other words, if a record no longer had a transaction pointer, it
> ended before any transaction that would be calling getRelativeState()
> ever started.
This implies that the code you are worried about is no longer used or
needed;
// Be sure it was not active when we started.
for (int n = 0; n < numberStates; ++n)
if (states [n].transactionId == transId)
return CommittedInvisible;
Can you run a test to see if that return is EVER hit in the current code?
>
>
>> 2) Comments in the code above seems to suggest that the reason for not
>> having the pointer to the transaction object is due to that the
>> transaction object has been deleted and it assumes that the
>> transaction has been committed. In my testing this seems not like it
>> is an correct assumption, in most cases when the transaction pointer
>> is NULL the transaction is actually still in Active state. Questions:
>>
>> a) Are the comments in the code wrong? Or
>> b) The code is wrong? And
>> c) Why do we not have the transaction pointer pointing to the
>> transaction if it is still active
>>
>> or (d) I have I introduced some error that causes this?
>
> You have introduced an error that causes Transaction::commitRecords() to
> be called while dependencies still exist.
>
> Kevin
>
>
>
