Skip to content

fix: Mark timed-out mutates and reset the timeout counter on answers - #207

Open
shy-1234 wants to merge 2 commits into
masterfrom
evcache-dead-node-impl
Open

shy-1234 wants to merge 2 commits into
masterfrom
evcache-dead-node-impl

Conversation

@shy-1234

Copy link
Copy Markdown
Contributor

What

Changes to EVCacheMemcachedClient.mutate (incr/decr):

  • When the mutate times out: it calls op.timeOut() and MemcachedConnection.opTimedOut(op), then returns -1. The ipc.client.call timer is tagged ipc.result=failure, ipc.status=timeout and evc.failed.host.
  • When the node answers with any status except timed out or cancelled: it calls MemcachedConnection.opSucceeded(op).

Why

  • No timeout metric: a timed-out mutate records its timer with no ipc.status or ipc.result tag, so mutate timeouts don't show up in metrics.
  • Timeouts not counted: mutate timeouts never add to the node's continuous-timeout counter.
  • Late writes: an op that times out while still queued can be sent, and applied, after the caller already got -1. Calling op.timeOut() makes the IO thread drop it.
  • Counter not reset: mutate answers never reset the counter, so on clients that mostly incr/decr it stops counting only consecutive timeouts.

The latch alone decides whether the mutate timed out, so a reply that arrives after the deadline can't change the result. The status holder is now an AtomicReference, because op.timeOut() runs the callback on the caller thread while the IO thread may run it too.

Testing

EVCacheMemcachedClientMutateTest has 6 tests and is added to test-suite.xml. It runs a real client against a fake memcached on 127.0.0.1 and covers:

  • a silent server: timeout timer tags and counter increment;
  • the counter counting 1, 2, 3 and then resetting on an answer;
  • an error answer resetting the counter;
  • a cancelled op leaving the counter unchanged;
  • a timed-out op that was never sent not reaching the server after reconnect;
  • an answered mutate returning the server's value.

Shih-Hao Yeh added 2 commits October 26, 2026 13:09
When the mutate latch times out, call op.timeOut() so an op still in
the write queue is dropped instead of being applied later, and feed
MemcachedConnection.opTimedOut so the node's continuous-timeout counter
counts it. The timer is tagged ipc.status=timeout with the failed host.

When the node answers (any status except timed out or cancelled), call
MemcachedConnection.opSucceeded so the counter stays consecutive on
clients that mostly incr/decr.

The latch result decides the outcome, so a reply racing in after the
deadline cannot flip it. The status holder is now an AtomicReference
because op.timeOut() runs the callback on the caller thread.
- main: use BaseOperationImpl.TIMED_OUT instead of a private duplicate,
  move the "latch decides" comment next to the status it explains, and
  null-guard the handling node when building the evc.failed.host tag
- tests: rewrite the unsent-op test so it fails without op.timeOut()
  (node operation timeout 60 s, assert the server saw only the second mutate)
- tests: add late-reply-races-the-timeout, first-status-wins and
  no-handling-node tests
- tests: rename the NF test, fix the TestConnectionFactory javadoc, drop a
  redundant null check, import SocketAddress and ArrayBlockingQueue, and give
  the canceller test a 2 s mutate timeout
@shy-1234 shy-1234 changed the title Mark timed-out mutates and reset the timeout counter on answers fix: Mark timed-out mutates and reset the timeout counter on answers Oct 27, 2026

This branch has not been deployed

No deployments
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