Remove the check in loadInventoryPage#8542
Conversation
hvlad
left a comment
There was a problem hiding this comment.
Description is too hard to read and understand, sorry.
We already discussed the issue with @dyemanov privately and it was much more clear.
I see no problem accepting this change if:
- it was carefully tested, and
- description should be improved and simplified, it is important for documentation purposes.
|
Ok, I'll describe more briefly. I'm not sure this will give a good understanding of the problem, but if more detail is needed, you can read the PR description. The error is that there is an invalid state for the transaction in the cache. Because of the same problem, another situation occurs: TPC_find_states analyzes the tip cache and does not recheck, because of this it can get an incorrect state How did this affect the tests:
|
An error related to the difference between the transaction states on disk and in the cache has occurred.
Because the tip cache is not loaded and a new block in the tip cache is initialized with CN_ACTIVE, the following can happen:
A record was added to a table in transaction 1. It is read in transaction 3. TipCache::snapshotState() calls TipCache::setState(), which sees the value CN_ACTIVE in the cache. Re-checks through the loc-manager and reads tra_committed from disk. A new commit number is generated, which is 3. As a result, stateCn = 3 is returned from TipCache::snapshotState(). This occurs when transaction 3 reads the RDB$TABLESPACES table (Calls TRA_snapshot_state). Then a check is triggered if (state == tra_committed && stateCn > trans->tra_snapshot_number) , since stateCn = 3 and trans->tra_snapshot_number = 2. And it ends up returning tra_active.
Although it should be tra_commited.
I can describe in more detail if it's not clear. I described it briefly.
There is also a problem:
The transaction_start function should move the OIT based on the check (--oldest > dbb->dbb->dbb_oldest_transaction)
Without a commit, the variable oldest, which participates in the check within the test, always remains at value “1”.
With commit, the variable oldest changes from “1” to another value after calling TPC_find_states .
It's not entirely clear to me what this method does, but from what I can see it's based on cache only and doesn't do any retests.
I went through it in the debugger and saw that without a commit for transaction “1” the state is considered “0”, that is, the transaction is active.
Then the variable oldest is updated by the condition of transaction completion. Since our transaction is considered active in the cache, the oldest variable does not move.
As a consequence, does not move and dbb_oldest_transaction.
Broken two tests that are actually fixed, I think.
core_4382_test.test_2 and gh_7208_test.test_1
When reading a table in a select * from g_test query , in VIO_chase_record_version the code reaches purge which will eventually call BTR_remove .
Without commit:
When reading a table in a select * from g_test query , in VIO_chase_record_version notify_garbage_collector is called to record .
So the difference is who deletes the old record, either immediately or in GC.
This difference happened because of the situation I described above with the retarded promotion of the oldest. The difference is obtained in transaction->tra_oldest_active . With check the value = 1, and without = 9. This affects the condition rpb->rpb_transaction_nr >= oldest_snapshot
The problem is that OAT is not updated correctly and stays at “1” for a while, even though the transaction has already been completed. Because of this it is considered still active and IMGC is called instead of PURGES.