Refactor request handler's doHandleRequest() into focused maintainable phases#163
Open
siddharthteotia wants to merge 1 commit into
Open
Refactor request handler's doHandleRequest() into focused maintainable phases#163siddharthteotia wants to merge 1 commit into
siddharthteotia wants to merge 1 commit into
Conversation
…ocused phases Extract five cohesive private methods from the 495-line doHandleRequest(): - prepareOfflineAndRealtimeRequests(): hybrid/offline/realtime branching, time-boundary attachment, expression/timestamp overrides, optimizer, always-false/true pruning - collectRoutingWarnings(): disabled table names and unavailable segment warnings - configureQueryTimeouts(): per-table timeout configuration (throws TimeoutException) - configureServerResponseOptions(): max response size and single-server final-result flag - finalizeResponse(): replica groups, exception propagation, metrics emission, query logging doHandleRequest() is now ~175 lines (down from ~495) with each phase clearly labeled. No behavioral changes; timing semantics preserved by capturing executionEndTimeNs before finalizeResponse(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
siddharthteotia
force-pushed
the
steotia/refactor-do-handle-request
branch
from
01f87bb to
081b818
Compare
siddharthteotia
changed the title
siddharthteotia
requested review from
UOETianleZhang,
adasari,
anujyshah and
justahuman1
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
1 participant
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.
Summary
JIRA: OA-850
Extracts five private methods from the 495-line
doHandleRequest(), reducing it to ~175 lines with each processing phase clearly labeledprepareOfflineAndRealtimeRequests()– hybrid/offline/realtime branching, time-boundary attachment, expression/timestamp overrides, query optimizer, always-false/true filter pruningcollectRoutingWarnings()– accumulates non-fatal routing warnings (partially-disabled tables, unavailable segments)configureQueryTimeouts()– per-table timeout configuration; letsTimeoutExceptionpropagate to the caller for clean handlingconfigureServerResponseOptions()– max serialized response size per server and the single-server final-result flagfinalizeResponse()– replica group metadata, exception propagation, phase metrics, query loggingNo behavioral changes.
Timing semantics are preserved:
executionEndTimeNsis captured viaSystem.nanoTime()indoHandleRequest()immediately afterprocessBrokerRequest()returns and passed intofinalizeResponse().Also cleaned up 5 now-unnecessary local variable unpacks from routeInfo at the top of the post-routing block
Test plan
All 14 existing broker request tests pass.
BaseSingleStageBrokerRequestHandlerTest– 5 tests passLiteralOnlyBrokerRequestTest– 8 tests passBrokerRequestOptionsTest– 1 test passesmvn compile -pl pinot-broker– clean compilation🤖 Generated with Claude Code