feat: wake io_uring workers through eventfd - #25
Conversation
71b2c51 to
de458c5
Compare
|
What's the motivation of this PR? If I understand correctly, it is "Let the owning brpc worker block in io_uring_submit_and_wait() instead of starting poll_thd_". But this is problematic. The reason is as follows: we use brpc worker thread as the worker thread to drive execution of all modules, including networking, runtime, tx service and storage. This worker cannot park in any module, but in a place that can be woken by any module if it has incoming tasks/requests/responses. Parking in the networking means that only the network event may wake it. What if there are cc requests combing from other cores for processing? as for the perf improvement, what I don't get is this: when the workload is high, the brpc worker thread should be busy pooling all modules. The backgroud thread of the networking module should never get the chance to wake up. This change won't make a difference, at least not better. If this change is better, it must be something else. btw, for the io uring, one lession I learned from Eloqstore is that because we use DEFER_TASKRUN, cqe may not arrive proactively in time. we need to frequently call io_uring_enter to get the result back. Is this relevant here? |
The motivation of this PR is to enable IORING_SETUP_DEFER_TASKRUN for the worker-owned io_uring. The performance improvement is not from replacing the condition variable with eventfd. I verified this with an ablation test: I kept SINGLE_ISSUER, eventfd, and the scheduler wakeup logic unchanged, and removed only DEFER_TASKRUN and TASKRUN_FLAG. The performance improvement disappeared. In the previous design, the brpc worker submitted requests while poll_thd_ waited for CQEs. That design does not require eventfd, but DEFER_TASKRUN requires io_uring_enter(GETEVENTS) to be called by the same thread that submitted the requests (io_uring_setup(2) (https://man7.org/linux/man-pages/man2/io_uring_setup.2.html)). Therefore, the brpc worker must own both submission and CQE waiting. Once the worker waits in io_uring_submit_and_wait(), a condition-variable notification cannot wake it. Eventfd is therefore registered on the same ring so that scheduler notifications from other modules generate a CQE and wake the worker. Before waiting, TaskGroup checks HasTask() for every registered module, and checks them again immediately before WaitForCqe(). If work arrives after the final check, NotifyWorker() eventually writes to the eventfd. Therefore, the worker is not waiting only for networking events. While the worker is active, io_uring_peek_cqe() uses IORING_SQ_TASKRUN to let liburing enter with GETEVENTS when deferred task work is pending. When idle, io_uring_submit_and_wait(..., 1) also enters with GETEVENTS. |
afc2e1b to
b4a9d86
Compare
What problem does this PR solve?
Issue Number: N/A
Problem Summary:
With
use_io_uring=true, each brpc worker currently owns a separate polling pthread. That pthread waits for CQEs and wakes the worker through a condition variable, adding one thread and a cross-thread handoff per worker and preventing deferred task execution from being driven solely by the ring's issuer.What is changed and the side effects?
Changed:
use_io_uringthe single switch for the optimized scheduler path; there is no separate eventfd-wakeup gflag.RingListenerand arm a persistent multishot poll request for scheduler wakeups.io_uring_submit_and_wait()instead of startingpoll_thd_.IORING_SETUP_SINGLE_ISSUER,IORING_SETUP_DEFER_TASKRUN, andIORING_SETUP_TASKRUN_FLAG.TaskGroup::Notify()andNotifyIfWaiting()through eventfd while retaining notification coalescing and lost-wakeup checks.use_io_uring=false.Side effects:
use_io_uringnow also requires kernel/liburing support for deferred task execution and multishot poll; initialization fails explicitly when those capabilities are unavailable.Material design decisions:
IORING_CQE_F_MOREfails fast because silently continuing could permit a future permanent worker sleep.EAGAINfrom eventfd write is success: a saturated eventfd is already readable and therefore already provides the required wakeup.Verification:
The current GitHub Actions compile, format/license, and Bazel jobs have passed; the unit-test job is still running at the time of this update. End-to-end EloqKV and EloqDoc matrices run in the linked dependency-validation PRs.
Performance benchmark (600.004 seconds, read-only GET workload):
Post-change CPU utilization was 4,248.587 seconds total: 637.575 seconds user and 3,611.012 seconds system. No pre-change CPU measurement was supplied, so no CPU improvement is claimed.
Additional rate-limited memtier samples:
Both rate-limited commands requested 600 seconds but were interrupted with Ctrl+C after unequal short durations. They are supporting observations, not a stable comparison.
Risk / rollback / reviewer focus:
use_io_uringto restore the legacy runtime path, or revert this PR to retain the previous io_uring polling-thread implementation.TaskGroup::Wait()/Notify(),RingListener::WaitForCqe()/HandleCqe(), andTaskControl::stop_and_join().Check List: