suvodeep-pyne opened a new pull request, #17861:
URL: https://github.com/apache/pinot/pull/17861

   ## Summary
   - **Fix unguarded `writeAndFlush()` in 
`ServerChannels.sendRequestWithoutLocking()`**: The write listener never 
checked `f.isSuccess()`, so on write failure `markRequestSent()` was called 
anyway (false positive), no channel close occurred (zombie channel), no 
logging/metrics, and queries waited for full timeout instead of failing fast.
   - **Add `f.isSuccess()` check**: On failure, logs the error, increments 
`NETTY_CONNECTION_SEND_REQUEST_FAILURES` metric, calls `markServerDown()` for 
fast query failure, and closes the channel to prevent zombies. On success, 
existing behavior is preserved.
   - **Add `NETTY_CONNECTION_SEND_REQUEST_FAILURES` broker metric** for 
observability of write failures.
   - This is the broker-side counterpart to the server-side fix in PR #17845 / 
#17854.
   
   ## Design Decisions
   - **`markServerDown()` over `markQueryFailed()`**: Race-safe — if server 
already responded, it's a no-op. The `_channel.close()` also triggers 
`channelInactive()` → `markServerDown()` for all in-flight queries. Double 
invocation is safe (idempotent).
   - **Wrap `f.cause()` in RuntimeException**: `markServerDown` expects 
`Exception` but `f.cause()` returns `Throwable` (could be `OutOfMemoryError`).
   - **`REQUESTS_SENT`/`BYTES_SENT` kept outside listener**: They're 
incremented synchronously before write completes. Moving them inside the 
success path would be a separate behavioral change.
   
   ## Test plan
   - [x] Added `testWriteFailureClosesChannelAndFailsQuery` — verifies channel 
close, markServerDown called, markRequestSent never called
   - [x] Added `testWriteSuccessMarksRequestSent` — verifies markRequestSent 
called, markServerDown never called, close never called
   - [x] Point-compiled pinot-common and pinot-core
   - [x] All ServerChannelsTest tests pass
   - [x] license:format and spotless:apply pass


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to