IGNITE-627 Fixed inconsistent value in near cache on concurrent update

[anton-vinogradov]

IGNITE-627

https://issues.apache.org/jira/browse/IGNITE-627

Problem

Near cache, atomic mode. A near node puts a value; almost simultaneously another client updates the same key. Due to message reordering the reader notification for the second update reaches the near node before the response to the node's own put. Result: the near cache is left with a stale value forever (get(k) returns the old value while the whole cluster sees the new one), and the reader subscription is silently dropped, so no future update ever repairs it.

Root cause

The near entry does not exist during the put's in-flight window. For a near cache "no entry" legitimately means "evicted → unsubscribe me", so the protocol cannot distinguish it from "not created yet":

• the overtaking reader update (newer value) finds no entry → it is dropped, and the near node reports "no entry" → the primary removes the reader;
• the late response then creates the entry from scratch → there is no version to compare against, so the stale value is installed unchecked.

The fix — one guarantee

The near entry exists, and cannot be evicted, for the whole duration of a near update. It is created empty and reserved (GridNearCacheEntry.reserveEviction() — the existing mechanism GridNearGetFuture already uses for reads) at future mapping time, before the request is sent; all reservations are released when the future completes, on every completion path.

Everything else follows from mechanisms that already exist:

• the overtaking reader update finds the entry and applies the newer value — and the reader is kept, since the node no longer replies "no entry";
• the late response is discarded by the existing version check (versions are assigned by the primary under the entry lock, so version order == apply order — delivery order stops mattering);
• the empty reserved entry is invisible to reads (a read through it still goes to the primary), so there are no side effects.

Implementation notes: the reservation is taken before the future is published via addAtomicFuture (a published future can be completed concurrently, and a reservation taken after completion would never be released); reserveNearCacheEntry is idempotent per future, so remap does not double-reserve. Response processing is not changed.

Before / after

Master:

sequenceDiagram
    participant N as Near node
    participant P as Primary node
    N->>P: put(k, 1)
    Note over P: value 1 (ver 1), near node registered as reader
    P--)N: response (ver 1) — delayed
    Note over P: concurrent put(k, 2) → ver 2
    P->>N: reader update (ver 2)
    Note over N: no entry → update dropped
    N--)P: "no entry"
    Note over P: reader removed — no future updates
    Note over N: late response creates entry with stale value 1
    Note over N: get(k) = 1 forever, cluster sees 2

Loading

With the fix:

sequenceDiagram
    participant N as Near node
    participant P as Primary node
    Note over N: mapping: entry created empty + reserveEviction()
    N->>P: put(k, 1)
    Note over P: value 1 (ver 1), near node registered as reader
    P--)N: response (ver 1) — delayed
    Note over P: concurrent put(k, 2) → ver 2
    P->>N: reader update (ver 2)
    Note over N: entry exists → value 2 (ver 2) applied, reader kept
    Note over N: late response: ver 1 < ver 2 → discarded by version check
    Note over N: releaseEviction() on future completion
    Note over N: get(k) = 2 — consistent with the cluster

Loading

Transactional part (separate race, same ticket)

In tx caches the reader was registered in GridDhtTxLocalAdapter.addEntry, i.e. before prepare acquired entry locks, so clearReaders of a concurrently finishing remove could wipe it. Registration is moved into GridDhtTxPrepareFuture.map(IgniteTxEntry), after onEntriesLocked(). GridNearTxLocal.addReader is a no-op, so near-local transactions are unaffected.

The reservation approach follows Semen Boikov's 2019 prototype (branches ignite-627 / ignite-627-tx), minimized.

Tests

• New deterministic race tests IgniteCacheAtomicProtocolTest.testNearEntryUpdateRace{Put,PutIfAbsent,Invoke,PutAll}: block GridNearAtomicUpdateResponse, update the same key from the primary, unblock, assert the near cache observes the fresh value. They fail on unpatched master (expected:<2> but was:<1>) and pass with the fix.
• Un-muted CacheNearReaderUpdateTest (hard fail() since 2016) and testPut[Remove]ConsistencyMultithreaded for near-enabled configurations — all pass, stable across repeated runs.
• Near cache regression set (readers, evictions, topology change, full IgniteCacheAtomicProtocolTest) — 92 tests, all green.

🤖 Generated with Claude Code

[anton-vinogradov]

Ignite PR Checker verdict · RunAll build 9205244 · 147 suites ran, 0 reused

⚠️ 1 broken suite(s) (no reliable run):

• Platform C++ CMake (Win x64 | Release): Number of tests 595 is 43% less than 1046 in build #18136

✅ No test blockers otherwise; 24 pre-existing/flaky filtered out.

⏳ Auto re-run #1 in progress — 1 broken suite(s) re-queued (attempt 1/2), ≈ settled by 21:11 MSK. This comment updates when they settle.

[zstan]

As i can see - this structure and all logic about it is acceptable only for caches with confidured eviction, otherwize it useless, if i`m right it need to be fixed.

[anton-vinogradov]

The reservation is needed regardless of a configured eviction policy. The empty near entry created for the in-flight window is removed via markObsoleteIfEmpty inside GridCacheEvictionManager.touch(), which runs before the if (!plcEnabled) return; check — i.e. independent of any eviction policy. reserveEviction() sets evictionDisabled(), and markObsolete0() early-returns false while it is set, which is exactly what keeps the placeholder alive for the whole operation.

Empirically, the testNearEntryUpdateRace* tests use a plain NearCacheConfiguration with no eviction policy and reproduce the bug on master / pass with the fix. The configured-eviction case is covered separately by testNearEvictionPolicyOnUpdates.

[zstan]

ok, i comment this line : //reservedEntries.put(key, entry);
and run assembly tests like :

    @Test
    public void test0() throws Exception {
        testNearEntryUpdateRacePut();
        afterTest();
        testNearEntryUpdateRacePutIfAbsent();
        afterTest();
        testNearEntryUpdateRaceInvoke();
        afterTest();
        testNearEntryUpdateRacePutAll();
    }

100 times tests are passed, thus i still miss - why we need such a structure if no eviction is configured ?

[anton-vinogradov]

Your change keeps entry.reserveEviction() and only drops the map put, so the near entry is still created and still reserved for the window — that's why the four race tests still pass: the placeholder exists and can't be removed while the reordered response is in flight.

What dropping the put actually breaks is the release: the reservation taken by reserveEviction() is never released (the map is what releaseNearCacheEntry / releaseNearCacheEntries iterate), so evictReservations stays > 0 and the entry becomes permanently non-evictable.

The four race tests don't notice because they configure no near eviction policy and only assert the final value — they never exercise eviction after the operation. testNearEvictionPolicyOnUpdates does, and with your change it fails:

expected:<3> but was:<100>

i.e. a FIFO(3) near cache can no longer evict down to its limit, because every entry is stuck reserved.

So reservedEntries isn't there to make the entry exist (that's entryExx) — it's the bookkeeping that releases the reservation once the operation completes, making the entry evictable again. For a cache without near eviction that release is effectively a no-op, but the code path is shared and for a cache with near eviction it's mandatory. (The reservation itself is still needed even without a configured policy: otherwise the empty near placeholder is removed via markObsoleteIfEmpty in touch() during the window and the race reappears.)

[anton-vinogradov]

The reservation blocks markObsolete(), not the eviction policy — and markObsolete() is how near entries get removed even with no policy configured: GridCacheEvictionManager.touch() removes empty entries via markObsoleteIfEmpty before the plcEnabled check, and the reader-became-backup branch in this PR relies on markObsolete too.

An unreleased reservation therefore makes the entry permanently unremovable — empty placeholders (DELETE, failed keys, remap) would just accumulate forever, policy or not. The map is what release iterates, so writing to it is simply pairing every reserve with its release. The expected:<3> but was:<100> failure shows the same effect through a policy only because it's easiest to observe there.

[anton-vinogradov]

Here is a cache with NO eviction policy at all — plain new NearCacheConfiguration<>(), nothing else: testNearDeleteHistoryPurgeWithoutEvictionPolicy (just pushed). With your line commented out it fails:

expected:<4> but was:<100>

Why: even without a policy, near entries do get removed — delete-history tombstones (the empty entries left by removes) are purged on remove-queue rollover via markObsoleteVersion → markObsolete0 (GridNearAtomicCache.onDeferredDelete → removeVersionedEntry), and markObsolete0 returns false while an eviction reservation is held. Skip the map write → release has nothing to release → the reservation is held forever → tombstones can never be purged → the near map grows unbounded (the test caps delete history at 4 and does 100 put+remove: healthy code keeps the map at 4, without the map write all 100 stay).

So the structure is not about the eviction policy — it is about markObsolete, the generic removal mechanism, which caches without any policy rely on too.

[zstan]

I see that you append a tests for: FULL_SYNC, PRIMARY_SYNC modes, but FULL_ASYNC still not covered, plz append a tests also for this mode.