Retry throttling not being respected with default service config

[JoaoVitorStein]

What version of gRPC-Java are you using?

1.63

What is your environment?

macOS 14.4.1
openjdk 17.0.1 2021-10-19 LTS
Gradle 8.5

What did you expect to see?

I'm setting a default service config with a retry policy and a retry throttling policy.
The retry throttling policy is not being respected and retries are always being executed.
Please check the following serviceConfig

What did you see instead?

Retries are always being executed

Steps to reproduce the bug

Check this repo, it reproduces this issue. You can check the steps to run it in the readme of that project.
This is a modified version of the retry example given in gRPC documentation, adding a retryThrottling policy to it.

As you can see, the retry throttling is being correctly configured in the defaultServiceConfig

With this configuration, I would expect that after 5 failed RPCs, retries would be throttled. However, all retries are executed in this case.

Possible root cause

After some investigation, I was able to figure out that the throttling policy is only being used if it comes from the NameResolver, check here. This happens because effectiveServiceConfig and lastServiceConfig are equal when you have a defaultServiceConfig and NameResolver doesnt retrieve a serviceConfig

I guess that we need to also set the transportProvider.throttle when instantiating the ManagedChannelImpl (here)

[JoaoVitorStein]

If this is not expected, I can open the PR with the fix.

[kannanjgithub]

We need to set the lastServiceConfig to null here when instantiating the ManagedChannelImpl so that the service config is actually applied during the name resolution result. Can you open a PR with this fix?

[JoaoVitorStein]

Sure, let me work on it.

[kannanjgithub]

A correction - We should just delete this line, so that lastService config remains EMPTY_SERVICE_CONFIG and gets assigned to the value from the NR callback.

[JoaoVitorStein]

Yep, will open the PR soon.
However I found a possible problem. With this approach, the throttle will still only be used if the serviceConfigLookup is set to true.

To solve this the only thing I can think of is setting the transportProvider.throttle in the constructor.

[ejona86]

We do want the default service config to be fully-functional when serviceConfigLookup=false. Yeah, I think that means the only easy approach is to initialize throttle in the constructor. (Good find, noticing serviceConfigLookup=false broke)

[JoaoVitorStein]

@ejona86 updated the PR with this change, however my CLA manager is currently OOO. I think I will get his approval next week.

[ejona86]

@JoaoVitorStein, okay, that's fine. When you are able to complete the CLA, make a comment on the PR. Even though the CLA will turn green, it modifies the existing comment so there's no email notification to us.

[ejona86]

Fixed by #11290