[Bug] Derive cache service identity from an explicit flag, not EnableKVSync - #2618
[Bug] Derive cache service identity from an explicit flag, not EnableKVSync#2618hamacekh wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces an IsGateway option to InitOptions to explicitly identify the gateway-plugins service. It refactors the service identification logic in InitWithOptions into a dedicated helper function serviceIdentity and adds comprehensive unit tests to verify its behavior. There are no review comments, so no additional feedback is provided.
|
The job log shows All six error lines are the same single failed read, reported once per importing file. This PR touches three files ( I don't have permission to re-run the job — could a maintainer trigger it? |
…KVSync InitWithOptions inferred the calling component from opts.EnableKVSync, so a gateway running with prefix-cache KV event sync disabled was classified as "metadata" and had enableGPUOptimizerTracing and enableModelGPUProfileCaching forced off. AIBRIX_GPU_OPTIMIZER_TRACING_FLAG=true was therefore silently discarded, initTraceCache never ran, and no aibrix:<model>_request_trace_<ts> keys were written -- leaving the GPU optimizer with no workload signal. EnableKVSync is a feature toggle, not an identity. Add an explicit IsGateway field to InitOptions, set it in cmd/plugins, and extract the classification into serviceIdentity() so it can be unit tested. The intended suppression for metadata-service and controller-manager is preserved. Note this also restores GPU profile caching on the gateway, which defaults to true but was being disabled by the same block. Signed-off-by: Jan Hamacek <8758068+hamacekh@users.noreply.github.com>
c94e443 to
c812309
Compare
|
I have rebased it in hope it might trigger the job without approval this time, but it didn't. Will need maintainer to do that for me. |
Pull Request Description
Adds an explicit
IsGatewayfield tocache.InitOptions, sets it incmd/plugins, and extracts the component classification inInitWithOptionsinto a unit-testableserviceIdentity(). Behaviour for metadata-service and controller-manager is unchanged.Why.
InitWithOptionsinferred the calling component fromopts.EnableKVSync— effectively assuming the gateway always has prefix-cache KV event sync enabled. It usually does not:cmd/pluginspassesEnableKVSync: kvSyncEnabled && remoteTokenizerEnabled, and both default to false. A default gateway was therefore classified"metadata", and that branch force-disablesenableGPUOptimizerTracing. SoAIBRIX_GPU_OPTIMIZER_TRACING_FLAG=truewas silently discarded,initTraceCachenever ran, noaibrix:<model>_request_trace_<ts>keys were written, and the GPU optimizer — with no workload signal — reported "scaled to minimum" indefinitely.Two side effects worth flagging.
EnableKVSync: truealone no longer implies"gateway"— harmless, ascmd/pluginsis the only caller in the tree that sets it and it now setsIsGateway: truetoo. And this restoresenableModelGPUProfileCachingon the gateway, which defaults totrueand was disabled by the same block.Testing.
TestServiceIdentitycovers all five classification cases.make testgreen ongolang:1.22(go fmt,go vet, full unit suite).Runtime verification. On a live cluster the gateway now logs
service="gateway"withenableGPUOptimizerTracing=true, the trace ticker starts, andaibrix:<model>_request_trace_<ts>keys appear in Redis with real bucket data that the GPU optimizer consumes. That build was v0.7.0 plus this patch while the PR targetsmain; the patched region is identical between them (cmd/plugins/main.gois the same blob, theInitOptions/InitWithOptionsbodies unchanged), so the behaviour transfers, though I have not run amainbuild on a cluster.Related Issues
Resolves: #1713, #1014
Important: Before submitting, please complete the description above and review the checklist below.
Contribution Guidelines (Expand for Details)
We appreciate your contribution to aibrix! To ensure a smooth review process and maintain high code quality, please adhere to the following guidelines:
Pull Request Title Format
Your PR title should start with one of these prefixes to indicate the nature of the change:
[Bug]: Corrections to existing functionality[CI]: Changes to build process or CI pipeline[Docs]: Updates or additions to documentation[API]: Modifications to aibrix's API or interface[CLI]: Changes or additions to the Command Line Interface[Misc]: For changes not covered above (use sparingly)Note: For changes spanning multiple categories, use multiple prefixes in order of importance.
Submission Checklist
By submitting this PR, you confirm that you've read these guidelines and your changes align with the project's contribution standards.