unwrap ForwardingSubchannel during Picks#12658
unwrap ForwardingSubchannel during Picks#12658shivaspeaks wants to merge 3 commits intogrpc:masterfrom
Conversation
There was a problem hiding this comment.
It looks like we don't need WeightedRoundRobinPicker specifically here, as we're just using the normal pick API. Let's avoid digging into the picker for the tests that don't need it. Which unfortunately, isn't many of the ones you've changed (most dig into the picker).
| PickResult result = delegate.pickSubchannel(args); | ||
| Subchannel subchannel = result.getSubchannel(); | ||
| if (subchannel != null) { | ||
| if (subchannel instanceof ClusterImplLbHelper.ClusterImplSubchannel) { |
There was a problem hiding this comment.
Move this into the next block. It is already checking that there is a subchannel, and isOk() is guaranteed true when there's a subchannel (so it could have just checked the subchannel).
There was a problem hiding this comment.
No, no. Yes, this is in our own tests, but digging in like this is pretty much always wrong. Let's add an accessor in testFixtures to get the delegate, similar to GracefulSwitchLoadBalancerAccessor
There was a problem hiding this comment.
There is no testFixtures for xds, should we create one? We can create OrcaOobUtilAccessor and put it within test/java/io/grpc/xds/orca forlder.
There was a problem hiding this comment.
I think this is the only other test that doesn't need the getWrrPicker()
2 participants
This PR ensures that Load Balancing (LB) policies unwrap
ForwardingSubchannelinstances before returning them in aPickResult. This change is a critical prerequisite for deletingSubchannelAttributes and removing the internalgetInternalSubchannel()API.