chore(facade): remove no-op io_event_ notifies in ioloop v2

[glevkovich]

Remove io_event_.notify() calls from ExecuteBatch() and ReplyBatch(). These are no-ops because both functions run synchronously on the same V2 fiber that is the sole waiter of io_event_. A notify issued before the fiber reaches await() is consumed by prepareWait()'s epoch snapshot and has no effect.

Cross-connection backpressure relief is handled separately by NotifyPipelineWaiters() in IoLoopV2() after ParseLoop() returns, so removing these does not affect backpressure signaling.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Remove no-op io_event_ notifications from IoLoopV2 batch paths
✨ Enhancement 🕐 10-20 Minutes

Description

• Remove redundant io_event_.notify() calls after ExecuteBatch() and ReplyBatch() in IoLoopV2.
• Clarify V1 vs V2 reply flushing behavior to avoid per-chunk sendmsg overhead.
• Preserve backpressure relief signaling via IoLoopV2 NotifyPipelineWaiters() flow.

Diagram

graph TD
  A["Connection::ExecuteBatch/ReplyBatch"] --> B["IoLoopV2 single fiber"] --> C["ParseLoop"] --> D["Execute/Reply batches"] --> E["IoLoopV2 flush/yield"] --> F["NotifyPipelineWaiters"]

Loading

High-Level Assessment

The PR’s approach is the right minimal fix: remove per-batch io_event_.notify() calls that cannot wake any other waiter in the IoLoopV2 single-fiber model, while keeping backpressure relief signaling centralized in IoLoopV2 (NotifyPipelineWaiters). Considered alternatives like moving the notify elsewhere or reworking io_event_ semantics, but those would add complexity without functional benefit given the described epoch/prepareWait behavior.

Files changed (1) +3 / -11

Refactor (1) +3 / -11

dragonfly_connection.ccRemove IoLoopV2 no-op io_event_ notifications and tighten reply flushing +3/-11

Remove IoLoopV2 no-op io_event_ notifications and tighten reply flushing

• Deletes io_event_.notify() calls from ExecuteBatch() and ReplyBatch() when running under IoLoopV2, since the single V2 fiber is the only waiter and notifications issued before await are effectively consumed. Reworks the ReplyBatch() conditional to only flush for V1 and adds clarifying comments about why V2 delegates flushing to IoLoopV2 for coalescing and syscall reduction.

src/facade/dragonfly_connection.cc

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

[augmentcode]

🤖 Augment PR Summary

Summary: Removes redundant io_event_.notify() calls from Connection::ExecuteBatch() and Connection::ReplyBatch() in IoLoop V2.

Why: These notifications are no-ops in the single-fiber V2 loop; cross-connection backpressure wakeups continue to be handled via NotifyPipelineWaiters() in IoLoopV2/ParseLoop.

_{🤖 Was this summary useful? React with 👍 or 👎}

[augmentcode]

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

[Copilot]

Pull request overview

This PR removes io_event_.notify() calls from Connection::ExecuteBatch() and Connection::ReplyBatch() for IoLoop V2, based on the fact that V2 runs these paths synchronously on the same fiber that later awaits on io_event_, making these notifies ineffective/no-op.

Changes:

• Removed V2-only io_event_.notify() calls from ExecuteBatch() and ReplyBatch().
• Kept V1 behavior to flush replies in ReplyBatch(), while clarifying via comments that V2 flushing is handled in IoLoopV2.