Skip to content

persistit: stabilize flaky MVCCPruneTest.testPruneAlternatingAbortedAndCommittedVersions#273

Merged
vharseko merged 1 commit into
OpenIdentityPlatform:masterfrom
vharseko:fix/flaky-mvccprune-272
Jul 12, 2026
Merged

persistit: stabilize flaky MVCCPruneTest.testPruneAlternatingAbortedAndCommittedVersions#273
vharseko merged 1 commit into
OpenIdentityPlatform:masterfrom
vharseko:fix/flaky-mvccprune-272

Conversation

@vharseko

Copy link
Copy Markdown
Member

Summary

Fixes the flaky MVCCPruneTest.testPruneAlternatingAbortedAndCommittedVersions, which intermittently failed on CI with:

com.persistit.MVCCPruneTest.testPruneAlternatingAbortedAndCommittedVersions:269
  stored versions expected:<4> but was:<3>

Root cause

The prune that runs inside every MVCC store (Exchange.storeInternal -> MVV.prune) decides whether to drop the previous committed version by calling TransactionIndex.hasConcurrentTransaction, which consults the ActiveTransactionCache:

  • stale cache (ceiling below the version's commit timestamp) — conservatively keeps the older committed/primordial version;
  • fresh cache — removes it.

The test hardcoded the stale-cache outcome (VERSIONS_NOW_REMOVED_BY_PRUNING_BEFORE_STORE = 2: only the two aborted versions pruned). That assumption holds only while the whole 5-transaction loop fits between two ticks of the background TXN_UPDATE poll task, which recomputes the cache every 10 ms (TransactionIndex.POLLING_TASK_INTERVAL). On a loaded CI runner a recompute lands mid-loop, the store-time prune drops an extra version (the primordial one or the older committed one), and the count assertion fails with 3 (or theoretically 2) instead of 4.

The race is unrelated to recent changes (the init-time settling added for #264 runs before any test data exists) — it dates back to the introduction of prune-before-store; slow runners just made it visible.

Fix

Drive pruning deterministically instead of relying on timer phase: settle the active transaction cache explicitly before every store in the loop, so the prune always sees fresh commit status. The surviving-version count then has a single deterministic value — 4 versions removed (two aborted + primordial + the older committed one), 2 surviving — regardless of any additional background recomputes, which are idempotent at that point. The final assertion of the test (2 stored versions after the explicit prune) is unaffected.

This mirrors the deterministic-settling approach used for the other persistit/core flakes (#254, #255, #256, #259, #261, #262, #266).

Testing

  • MVCCPruneTest full class under JDK 11: 14/14 green.
  • Target method run 5 additional times in isolation: green.

Fixes #272.

…ndCommittedVersions (OpenIdentityPlatform#272)

The store-time prune in Exchange consults the ActiveTransactionCache: a
stale cache conservatively keeps the previous committed version while a
fresh one removes it. The test hardcoded the stale-cache outcome
(2 versions removed), which only holds while the whole store loop fits
between two ticks of the 10 ms TXN_UPDATE poll task. Under CI load a
recompute lands mid-loop, an extra version is pruned and the assertion
fails with "stored versions expected:<4> but was:<3>".

Settle the active transaction cache explicitly before every store so
pruning always sees fresh commit status, and update the expected count
to the deterministic fully-pruned outcome (4 versions removed, 2
surviving).

Fixes OpenIdentityPlatform#272.
@vharseko vharseko added the tests Test code changes label Jul 12, 2026
@vharseko vharseko requested a review from maximthomas July 12, 2026 06:57
}

@Test
public void testPruneRunOfAbortedAndCommittedVersions() throws PersistitException {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this test has the same issue, so the same fix could be applied here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good thought, but this test doesn't have the same race. The racy branch in MVV.prune (hasConcurrentTransaction(lastVersionTc, tc), MVV.java:472) only fires when two committed versions (or primordial + committed) coexist in the MVV during an in-store prune. Here the key starts non-existent (no primordial) and the second commit (v6) happens after the last store, so every in-loop prune sees at most one committed version — and the last committed version is always kept unconditionally. Aborted versions are pruned unconditionally regardless of the cache (only tc >= 0 versions get marked in the keep pass), so the pre-prune count is deterministically 2. The final explicit prune goes through the test helper, which calls TransactionIndex.cleanup() first — and that starts with updateActiveTransactionCache(), so the post-prune assertion is deterministic too. That also matches CI history: only the alternating variant has ever flaked (#272).

@vharseko vharseko requested a review from maximthomas July 12, 2026 09:43
@vharseko vharseko merged commit b1f81a1 into OpenIdentityPlatform:master Jul 12, 2026
14 checks passed
@vharseko vharseko deleted the fix/flaky-mvccprune-272 branch July 12, 2026 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tests Test code changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky test: MVCCPruneTest.testPruneAlternatingAbortedAndCommittedVersions (stored versions expected:<4> but was:<3>)

2 participants