Hand rolling#15
Open
Ataba29 wants to merge 10 commits into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Replaces the thread-per-connection model with a cross-platform,
readiness-based event loop (epoll on Linux, IOCP on Windows), unified
behind an
IEventLoopinterface.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
IEventLoopinterface withEpollEventLoop(Linux) andIocpEventLoop(Windows) implementations, selected at compile time via
EventLoopFactory::makeEventLoop()Connectionstruct +Server::connectionsmap, keyed by socketacceptClients()now sets sockets non-blocking and registers themwith the event loop instead of spawning a thread per client
messageHandler()now does onerecv()per event instead of looping,and correctly treats
EWOULDBLOCKas "nothing to do" rather than adisconnect
busySocketsguard to stop duplicaterecv()jobs from beingqueued 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, sograceful shutdown (
stop()) doesn't hang waiting on client activityisAllowed()happened.Testing
Manually tested with multiple concurrent
ncatclients: INSERT/GET/DELETEall confirmed working with 2+ simultaneous connections, no spurious
disconnects. Graceful
stopverified to return promptly with clientsstill 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 whena session idle-times-out, without telling
Server— soconnectionsandthe event loop can end up referencing a socket that's already been closed
elsewhere. Needs to be routed through
Server::closeConnection()for asingle consistent teardown path. Filing as a separate follow-up rather
than expanding scope here.