Skip to content

Fix Clear timestamps moving backwards - #658

Open
GordeySt wants to merge 1 commit into
ZiggyCreatures:mainfrom
GordeySt:fix/clear-timestamp-rollback
Open

GordeySt wants to merge 1 commit into
ZiggyCreatures:mainfrom
GordeySt:fix/clear-timestamp-rollback

Conversation

@GordeySt

@GordeySt GordeySt commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Hi @jodydonetti! First of all, congrats on your new course on caching. And second, let me walk you through an issue I found ☺️

ClearRemoveTimestamp and ClearExpireTimestamp can move backwards. When that happens, a node keeps serving entries that were already cleared, and with a backplane it does not recover on its own, because the field is loaded once and then only updated by notifications

These changes make both fields monotonic: every writer now goes through a small helper that only moves the value forward

Why was it happening? Every writer of these two fields used Interlocked.Exchange. That makes each write atomic, which is definitely better than a plain assignment, but it is unconditional: the last writer wins, even when its value is older

The main case I found is with two nodes C1 and C2 sharing an L2 and a backplane, where C2 has just started (its fields are at -1):

  1. A read on C2 starts the lazy load of the Clear marker and gets the old value from L2 (say 0).
  2. While that load is in flight, C1 calls Clear(). It writes the new timestamp T to L2 and publishes it.
  3. C2's backplane handler sets the field to T.
  4. C2's lazy load completes and runs Interlocked.Exchange(ref field, 0). T is gone.
  5. The field is now >= 0 and C2 has a backplane, so it is never reloaded

From here C2 serves every pre-Clear entry until some later Clear arrives. I reproduced it with deterministic tests: ClearRemoveDuringColdStartIsNotLostAsync and ClearExpireDuringColdStartIsNotLostAsync

The other scenarios in short:

  1. Out-of-order notifications from the backplane: two nodes clear close together, and the older notification reaches a third node last, lowering its field. Covered by OutOfOrderClearRemoveNotificationsAreNotLostAsync and OutOfOrderClearExpireNotificationsAreNotLostAsync
  2. A local Clear lowers a newer remote one: C2 receives a remote Clear with timestamp T, then calls its own Clear(), whose now is smaller than T, and the field goes down. This can happen with clock skew between nodes, or when the local call is paused right after taking its timestamp. Covered by LocalClearRemoveDoesNotUndoNewerRemoteClearAsync and LocalClearExpireDoesNotUndoNewerRemoteClearAsync

The fix I propose is a new helper, FusionCacheInternalUtils.AdvanceTimestamp(ref long, long): a compare-and-swap loop that writes only if the new timestamp is greater than the current one, and returns the previous value

It keeps what Interlocked.Exchange provided (atomic write, previous value for the trace log) and adds the "only move forward" rule. A larger Clear timestamp covers everything a smaller one does, so keeping the maximum can never lose a Clear

All writers of the two fields now use it, in both sync and async: the lazy loads, the raw-clear seed, the local Clear, and the backplane handler. The "new Clear timestamp" trace log condition changed from != to <, so it logs only when the value actually advanced

@GordeySt
GordeySt force-pushed the fix/clear-timestamp-rollback branch from 3ceb68f to a1647fd Compare October 5, 2026 16:06
@GordeySt

GordeySt commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor Author

One more thing to be upfront about scope, this PR makes the in-memory Clear timestamps monotonic, but the L2 copy of the Clear is still last-writer-wins

So L2 can still go backwards in a few cases:

  1. Two nodes call Clear() at nearly the same time and the older write lands last
  2. The same clock skew as in in-memory cache: a node calls Clear() with a now lower than a Clear it already recieved from another node, and writes that lower value to L2

But the impact is much smaller then the in-memory issue I am trying to fix via this PR. Running nodes with a backplane never re-read L2 and, thanks to this PR, keep the max they have seen. Only a node that starts fresh (or one without a backplane) can load the lower value, For the cases I described above that only matters for entries written between the two timestamps which is typically a window of milliseconds or the size of the clock skew

I didn't try to fix this here because IDistributedCache has no compare-and-swap, so a true "atomic max" in L2 isn't possible without provider-specific code

If you, as an author of the library, think it's worth it, I'd suggest we create an issue and try to come up with some nice solution in a separate PR not to widen the scope of the existing one and to properly discuss all the possible solutions with all the trade-offs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant