Add test case to verify freelist in case of TXN rollback #796
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahrtr The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| } | ||
| return nil | ||
| }) | ||
| require.NoError(t, err) |
There was a problem hiding this comment.
can you add an assertion to count the leafs and free pages here? just in case the numbers change in the future - or below, where you're reading the freelist pgids
There was a problem hiding this comment.
Added one more check below. We are good as long as the freelist ID isn't empty.
require.Greater(t, len(beforeFreelistPgids), 0)
tests/failpoint/db_failpoint_test.go
Outdated
| require.Equal(t, beforeFreelistPgids, afterFreelistPgids) | ||
|
|
||
| t.Log("Disable the `beforeWriteMetaError` failpoint.") | ||
| require.NoError(t, gofail.Disable("beforeWriteMetaError")) |
There was a problem hiding this comment.
that's probably a better candidate for a defer
There was a problem hiding this comment.
If we use defer, then we have to remove the require.NoError, something like below,
defer gofail.Disable("beforeWriteMetaError")
Note each time a "defer" statement executes, the function value and parameters to the call are evaluated as usual and saved anew but the actual function is not invoked.
There was a problem hiding this comment.
we don't have to remove the require to use it in a defer statement, but I'm fine with keeping it the way it is. I'm mostly concerned about a failure in this test impacting the other testcases with the failpoint and then one has to check which one caused the failure to begin with.
It's a separate suite with a few tests, so let's leave it
There was a problem hiding this comment.
Updated again,
defer func() {
t.Log("Disable the `beforeWriteMetaError` failpoint.")
require.NoError(t, gofail.Disable("beforeWriteMetaError"))
}()
ahrtr
force-pushed
the
rollback_test_20240717
branch
from
16fd74f
to
63380a8
Compare
|
/lgtm |
Signed-off-by: Benjamin Wang <[email protected]>
ahrtr
force-pushed
the
rollback_test_20240717
branch
from
63380a8
to
7b031d5
Compare
|
thanks! /lgtm |
The freelist should be unchanged when TXN rollbacks.