Skip to content

Hand rolling#15

Open
Ataba29 wants to merge 12 commits into
mainfrom
handRolling
Open

Hand rolling#15
Ataba29 wants to merge 12 commits into
mainfrom
handRolling

Conversation

@Ataba29

@Ataba29 Ataba29 commented Jul 7, 2026

Copy link
Copy Markdown
Owner

What

Replaces the thread-per-connection model with a cross-platform,
readiness-based event loop (epoll on Linux, IOCP on Windows), unified
behind an IEventLoop interface.

Why

Previously, every connected client permanently occupied one thread-pool
worker for its entire lifetime, capping concurrent connections at the
pool size (3) regardless of how idle those clients were. Now a small
number of threads watch all sockets for readiness; only actual command
execution (GET/INSERT/DELETE) uses the thread pool.

Changes

  • IEventLoop interface with EpollEventLoop (Linux) and IocpEventLoop
    (Windows) implementations, selected at compile time via
    EventLoopFactory::makeEventLoop()
  • Connection struct + Server::connections map, keyed by socket
  • acceptClients() now sets sockets non-blocking and registers them
    with the event loop instead of spawning a thread per client
  • messageHandler() now does one recv() per event instead of looping,
    and correctly treats EWOULDBLOCK as "nothing to do" rather than a
    disconnect
  • Added a busySockets guard to stop duplicate recv() jobs from being
    queued for the same socket before a prior job finishes (level-triggered
    epoll can otherwise report the same socket ready multiple times before
    it's drained)
  • wait() now times out after 1s instead of blocking forever, so
    graceful shutdown (stop()) doesn't hang waiting on client activity
  • Fixed test_ratelimiter.cpp, as in previous updates a change to the isAllowed() happened.

Testing

Manually tested with multiple concurrent ncat clients: INSERT/GET/DELETE
all confirmed working with 2+ simultaneous connections, no spurious
disconnects. Graceful stop verified to return promptly with clients
still connected, as well as running unit tests and having all of the 16 tests pass.

Known follow-up (not in this PR)

UserSession::terminate_session() closes the client socket directly when
a session idle-times-out, without telling Server — so connections and
the event loop can end up referencing a socket that's already been closed
elsewhere. Needs to be routed through Server::closeConnection() for a
single consistent teardown path. Filing as a separate follow-up rather
than expanding scope here.

@Ataba29 Ataba29 requested a review from razimograbi July 7, 2026 11:06
@Ataba29

Ataba29 commented Jul 12, 2026

Copy link
Copy Markdown
Owner Author

Ran stress test and here are the results

Metric 200 clients 2,000 clients 10,000 clients
Connected 200/200 2,000/2,000 10,000/10,000
Requests completed 4,000/4,000 40,000/40,000 199,980/200,000
Throughput 11,908 req/sec 10,401 req/sec 8,207 req/sec
p95 latency 15.5 ms 190.7 ms 1,199 ms
Errors 0 0 1 timeout

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