Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

shard_connection: Handle non-blocking I/O errors on Unix socket client connect #258

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LINKIWI
Copy link

@LINKIWI LINKIWI commented Jun 16, 2024

Currently, the client socket is configured as non-blocking, but non-blocking I/O errors are not properly handled on connect(2). (Reference)

This seems to be problematic for Unix socket clients, where a restrictive server TCP listen backlog (or a restrictive system setting for net.core.somaxconn) relative to the number of concurrent memtier_benchmark clients (--clients) results in persistent benchmark execution failures with Resource temporarily unavailable.

A simple reproduction case:

# Artifically restrict the listen(2) backlog queue size.
$ echo -en 'port 0\nunixsocket /tmp/redis.sock\ntcp-backlog 1\n' | ./redis-server -
...
$ ./memtier_benchmark -S /tmp/redis.sock --clients 100
Writing results to stdout
[RUN #1] Preparing benchmark client...
connect failed, error = Resource temporarily unavailable
prepare: failed to connect, test aborted.
error: failed to prepare thread 0 for test.

This patch augments the connect(2) path to handle EINPROGRESS and EAGAIN/EWOULDBLOCK errors; the connect is simply retried indefinitely for the latter case.

With this patch:

$ ./memtier_benchmark -S /tmp/redis.sock --clients 100
Writing results to stdout
[RUN #1] Preparing benchmark client...
[RUN #1] Launching threads now...
^CUN #1 27%,   2 secs]  4 threads:     1078250 ops,  359004 (avg:  359421) ops/sec, 14.87MB/sec (avg: 14.89MB/sec),  1.11 (avg:  1.11) msec latency
...

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