[DO NOT MERGE / test bundle] resourcecontrol: PredictedReadBytes hint for paging pre-charge#1968
[DO NOT MERGE / test bundle] resourcecontrol: PredictedReadBytes hint for paging pre-charge#1968YuhaoZhang00 wants to merge 9 commits into
Conversation
Add a client-go-internal PredictedReadBytes field on tikvrpc.Request so the caller (e.g. TiDB, maintaining a per-logical-scan EMA across paging cop RPCs) can supply a learned estimate of how many bytes the request will read. MakeRequestInfo propagates this into RequestInfo, and the new RequestInfo.PredictedReadBytes() getter satisfies the optional predictedReadBytesProvider interface that PD's resource_group/controller checks via type assertion. When the hint is > 0, PD uses it as the byte basis for RC paging pre-charge. Zero means the caller has no prediction (e.g. cold start); the request is not pre-charged and is billed at settlement time by actual read bytes only. The field is kept out of the proto because it is purely a client-side estimate consumed before the RPC is sent - TiKV neither needs nor reads it. Signed-off-by: Yuhao Zhang <yhzhang00@outlook.com>
Drop the verbose restatement of how the hint is produced and consumed; keep only what a reader of this struct needs to know to set the field. Signed-off-by: Yuhao Zhang <yhzhang00@outlook.com>
Signed-off-by: Yuhao Zhang <yhzhang00@outlook.com>
PD promoted PredictedReadBytes to a required method on RequestInfo, so this is no longer an optional duck-typed satisfaction. Signed-off-by: Yuhao Zhang <yhzhang00@outlook.com>
ti-chi-bot
Bot
added
do-not-merge/work-in-progress
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ti-chi-bot
Bot
added
contribution
PD's resource-control controller needs to gate paging_* accounting on coprocessor requests only; non-cop reads (CmdGet, CmdBatchGet, CmdScan, internal lookups) reach the same interceptor but never participate in the EMA pre-charge path and must not appear in paging_nonprecharge_*. Add an isCop field on RequestInfo derived from tikvrpc.Request.Type at construction time, and expose it via IsCop() to satisfy PD's new RequestInfo interface method. Signed-off-by: Yuhao Zhang <yhzhang00@outlook.com>
interceptedClient.SendRequest and SendRequestAsync today call OnResponseWait only when the underlying RPC returns a non-nil response. That made sense in the legacy billing model (only base RRU / WRU at risk on transport failure) but became a real RU leak after the PD-side paging pre-charge: the predicted RU debited in OnRequestWait stayed permanently charged whenever the RPC failed before producing a response (connection drop, timeout, context cancellation). When resp == nil, invoke the new ResourceGroupKVInterceptor.OnRequestCancel so PD refunds the predicted RU to the limiter and rolls back the consumption row it added in OnRequestWait. Cancel and settlement are mutually exclusive on a given request — the if/else structure makes that invariant explicit at every call site. Pin the PD client replace to the matching tikv/pd#10611 commit; the replace will be removed once that PR is merged and tagged. Adds TestSendRequest{Cancels,DoesNotCancel}{,Async}* covering the three combinations (sync failure / sync success / async failure) through a recordingInterceptor mock so the wiring is verified end-to-end without touching a real PD. Signed-off-by: Yuhao Zhang <yhzhang00@outlook.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
ti-chi-bot
Bot
added
dco-signoff: no
|
Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits. 📝 Please follow instructions in the contributing guide to update your commits with the DCO Full details of the Developer Certificate of Origin can be found at developercertificate.org. The list of commits missing DCO signoff:
DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
ti-chi-bot
Bot
added
size/XL
1 participant
Draft / test-bundle PR for the
rc-precharge-classic-testintegration bundle (classic / non-Premium variant of the RC paging pre-charge feature).Summary
resourcecontrol: addPredictedReadByteshint toRequestInfotikvrpc: tightenPredictedReadBytesfield commentSibling PRs (test bundle)
rc-precharge-classic-testrc-precharge-classic-testrc-precharge-classic-testrc-precharge-classic-testThis branch exists for upstream CI / cross-repo integration testing. Not intended to be merged as-is.