Skip to content

Add RPCObserver interface to encapsulate stats handling; Record ScanStats start time after congestion queue#338

Merged
aaronbee merged 3 commits intomasterfrom
rpcobserver
Apr 14, 2026
Merged

Add RPCObserver interface to encapsulate stats handling; Record ScanStats start time after congestion queue#338
aaronbee merged 3 commits intomasterfrom
rpcobserver

Conversation

@aaronbee
Copy link
Copy Markdown
Collaborator

@aaronbee aaronbee commented Apr 13, 2026

This fixes an issue where the duration of a scan request includes the time it is queued within gohbase waiting for the congestion control to release the scan.

@aaronbee
Copy link
Copy Markdown
Collaborator Author

Posting this as a draft for now. I still need to review the code the AI agent produced.

@aaronbee aaronbee force-pushed the rpcobserver branch 2 times, most recently from be848cc to 87b494e Compare April 13, 2026 21:48
@aaronbee aaronbee requested a review from ciacono April 13, 2026 21:48
@aaronbee aaronbee marked this pull request as ready for review April 13, 2026 21:48
Introduce RPCObserver, an optional interface that RPCs can implement to
track per-attempt timing and completion statistics. Scan implements it
by moving the stats-building logic (previously in client.scanRpcScanStats)
into Scan.SetSendTime and Scan.OnComplete methods. This encapsulates
scan stats handling within the Scan type rather than requiring the
top-level SendRPC function to have scan-specific knowledge.
Record the RPC send time right after registerRPC() returns, which is
after the congestion control token bucket releases the RPC. This gives
a more accurate start time than the previous approach of capturing it
in SendRPC before the RPC is queued, since the token bucket can delay
scans arbitrarily.
Replace four scan type assertions in SendRPC with a single RPCObserver
interface check. Remove scanRpcScanStats and the pb import it required.
The scan stats callback is now triggered through the RPCObserver
interface, so SendRPC no longer needs to know about scan RPCs.

Update TestScanRPCScanStatsScanMetricsNonScanResponse to call
Scan.OnComplete directly instead of the removed scanRpcScanStats.
@aaronbee aaronbee merged commit 1db549e into master Apr 14, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants