xiangfu0 commented on PR #20023: URL: https://github.com/apache/pinot/pull/20023#issuecomment-6400580402
**Code review verdict: approve (no blocking issues).** Reviewed at `b8db9d3`. Covered broker/server request handling, deadline stamping, concurrency, and backward compatibility per the repo's review guidelines. - **Correctness:** the broker stamps `now + remainingTimeMs` (max of the two sides for hybrid) on every dispatched single-stage request before dispatch, and the MV-split path re-stamps the rebuilt base + view requests with a fresh budget. Server-side `rejectIfExpired` gates on arrival only, overflow-safe, and leaves the server's own `timeoutMs` to govern execution. No path dispatches without a deadline, and every null request object is handled. - **Back-compat:** absent / non-positive `requestDeadlineMs` reproduces old-broker behavior; a user value is always overwritten; a malformed value (only reachable via an old broker forwarding user input) returns a parse error under the right request id. Binary-compat CI is green. - **Concurrency:** the rate-limited expired-request logger's atomics are sound — no increment is lost across the CAS; the meter is exact. - **Docs/tests:** excellent Javadoc (including the honest note that the stamp predates `AsyncQueryResponse`'s start, so the server is marginally stricter than the broker's real wait, covered by the clock-skew allowance). Thorough coverage across unit, Netty end-to-end, and hybrid/MV-split paths. All CI checks pass. Could not click the formal Approve button because this account authored the PR (GitHub blocks self-approval), but from a review standpoint this LGTM. One small appreciative note left inline. -- 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]
