Enable compiled endpoint rules for all services and fix regionId parameter mismatch bug - #7265
Open
S-Saranya1 wants to merge 3 commits into
Open
Enable compiled endpoint rules for all services and fix regionId parameter mismatch bug#7265S-Saranya1 wants to merge 3 commits into
S-Saranya1 wants to merge 3 commits into
Conversation
…meter mismatch bug
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
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.
Motivation and Context
Compiled endpoint rules generate endpoint resolution logic as direct Java code at codegen time, rather than interpreting rules at runtime. This provides ~40% better performance. The feature was behind a flag (
enableGenerateCompiledEndpointRules) and already enabled for 344 services externally. This PR enables it for all remaining internal services by flipping the flag totrue.However, simply flipping the flag exposed a bug in the compiled endpoint provider codegen. The resolveEndpoint method extracts the region as a local String variable and always passes it to the root rule method:
But the root rule method's signature is generated based on scope analysis, it only includes regionId as a parameter if that method directly uses it. For most services, the root rule directly uses region (e.g., to look up the partition), so the 2-arg call matches:
private static RuleResult endpointRule0(Params params, String regionId) { ... }// 2 params - worksBut for some services, the root rule just delegates to a child rule without using region itself:
private static RuleResult endpointRule0(Params params) { ... }// 1 param - compile error!This creates a mismatch: the call site passes 2 arguments but the method only accepts 1.
This PR fixes the bug by adding a regionId() method to the endpoint params class that returns the region as a String (null-safe). The codegen now emits params.regionId() wherever the rules need the region as a string, eliminating the local variable and the parameter passing mismatch entirely.
Modifications
enableGenerateCompiledEndpointRulesdefault from false to trueRegion region = params.region();String regionId = ...) fromresolveEndpointMethod(). UpdateinitSymbolTable()to mark region params viaaddRegionParam()instead of creating a localString regionParamNamewithSet<String> regionParams. AddisRegionParam()andaddRegionParam()methodsregionParamName()accessorparams.regionId()(append "Id" to accessor name) instead ofparams.region()regionId()method for Region-typed params:return region == null ? null : region.id()regionId()default method to the interfaceregionId()overriderulesEngineResourceFiles2()so compiled rules codegen works when running from exploded classes in test environmentsTesting
params.regionId()patternScreenshots (if appropriate)
Types of changes
Checklist
mvn installsucceedsscripts/new-changescript and following the instructions. Commit the new file created by the script in.changes/next-releasewith your changes.License