Optimize linear search of WAIT and WAITAOF when unblocking the client

[enjoy-binbin]

Currently, if the client enters a blocked state, it will be
added to the server.clients_waiting_acks list. When the client
is unblocked, that is, when unblockClient is called, we will
need to linearly traverse server.clients_waiting_acks to delete
the client, and this search is O(N).

When WAIT (or WAITAOF) is used extensively in some cases, this
O(N) search may be time-consuming. We can remember the list node
and store it in the blockingState struct and it can avoid the
linear search in unblockClientWaitingReplicas.

[enjoy-binbin]

An extreme test (wait 10 make sure it will get blocked, and the timeout is just 1ms so it can get unblocked in a short time)

./src/valkey-benchmark -c 5000 -n 1000000 wait 10 1

before:

Summary:
  throughput summary: 52452.14 requests per second
  latency summary (msec):
          avg       min       p50       p95       p99       max
       85.334    27.792    80.575   153.215   166.911   190.591

Summary:
  throughput summary: 52645.43 requests per second
  latency summary (msec):
          avg       min       p50       p95       p99       max
       84.970    27.888    80.383   154.751   164.991   185.215

Summary:
  throughput summary: 51894.13 requests per second
  latency summary (msec):
          avg       min       p50       p95       p99       max
       86.001    29.632    80.767   153.215   163.583   193.279

after:

Summary:
  throughput summary: 61881.19 requests per second
  latency summary (msec):
          avg       min       p50       p95       p99       max
       41.361    21.680    41.567    44.095    45.599    58.559

Summary:
  throughput summary: 62386.93 requests per second
  latency summary (msec):
          avg       min       p50       p95       p99       max
       41.008    19.872    41.247    43.455    45.087    54.143

Summary:
  throughput summary: 61606.70 requests per second
  latency summary (msec):
          avg       min       p50       p95       p99       max
       41.515    19.424    41.695    43.839    45.631    54.943

[zuiderkwast]

Please mention the user-visible changes in PR description. Optimization of WAIT and WAITAOF?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.26%. Comparing base (418901d) to head (c88d5f9).
Report is 61 commits behind head on unstable.

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable     #787      +/-   ##
============================================
- Coverage     70.28%   70.26%   -0.02%     
============================================
  Files           112      112              
  Lines         60587    60592       +5     
============================================
- Hits          42581    42575       -6     
- Misses        18006    18017      +11     

Files	Coverage Δ
src/blocked.c	91.60% <100.00%> (+0.14%)	⬆️
src/replication.c	87.23% <100.00%> (ø)
src/server.h	100.00% <ø> (ø)

... and 10 files with indirect coverage changes

[ranshid]

Thank you @enjoy-binbin. this optimization looks correct!

I would only suggest that we try and encapsulate all blocking related data in the BlockingState struct in the client.
Currently we keep the:

/* BLOCKED_WAIT and BLOCKED_WAITAOF */
    int numreplicas;      /* Number of replicas we are waiting for ACK. */
    int numlocal;         /* Indication if WAITAOF is waiting for local fsync. */
    long long reploffset; /* Replication offset to reach. */

inside the blocking state, maybe adding the list node ptr there is cleaner?

[ranshid]

LGTM

[zuiderkwast]

Looks correct. Only question: Is it worth 8 bytes per client?

I guess it's OK but in the future, we can try to optimize the memory of the client struct. We could change this kind of lists to dicts or something else where we don't need to store a list node.

[enjoy-binbin]

I guess it's OK but in the future, we can try to optimize the memory of the client struct. We could change this kind of lists to dicts or something else where we don't need to store a list node.

postponed_list_node also do the same thing, i think it is ok. yes, we can optimize it in the future.