io.grpc.LoadBalancer method signatures don't match javadoc

[jdcormie]

javadoc for public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses)

• refers to non-existent argument servers

javadoc for public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses)

• refers to an argument named addresses
• refers to returning values not of the actual return type

Readers are probably also curious about the difference between "accept" and "handle" and which one implementations are expected to override.

[kannanjgithub]

Readers are probably also curious about the difference between "accept" and "handle" and which one implementations are expected to override.

I think acceptResolvedAddresses is the one implementations should prefer to use because it returns a status of the acceptance back to the name resolution system. However handleResolvedAddresses is internally still used in the multi level Xds load balancers, which is probably why it is not deprecated. @ejona86 please confirm.

[ejona86]

handleResolvedAddresses is internally still used in the multi level Xds load balancers

That should be updated to call the new method.

why it is not deprecated

It isn't deprecated because we needed to make the StatusOr<addresses> change to the LB API and it wasn't clear at the time whether that would be API compatible. We didn't want people to move over to acceptResolvedAddresses() only for us to then create a acceptResolvedAddresses2() for the addresses part. At this point, it seems fine for it to be deprecated.

[SreeramdasLavanya]

handleResolvedAddresses is internally still used in the multi level Xds load balancers

That should be updated to call the new method.

why it is not deprecated

It isn't deprecated because we needed to make the StatusOr<addresses> change to the LB API and it wasn't clear at the time whether that would be API compatible. We didn't want people to move over to acceptResolvedAddresses() only for us to then create a acceptResolvedAddresses2() for the addresses part. At this point, it seems fine for it to be deprecated.

@kannanjgithub @ejona86

As per our understanding I'm marking handleResolvedAddresses method as deprecated and updating the references(14 usages of handleResolvedAddresses method).

[SreeramdasLavanya]

@ejona86 @kannanjgithub

While marking handleResolvedAddresses method as deprecated we observed that following cyclic calls in LoadBalancer as follows I.e., handleResolvedAddresses -> calls acceptResolvedAddresses -> calls handleResolvedAddresses which we are trying to mark as deprecated. Also not sure about significance of recursionCount, at this point require suggestions it would be helpful to proceed further.

[larry-safran]

recursionCount is used to break the cycle you've observed as in both handleResolvedAddresses and acceptResolvedAddresses the recursion count is checked to make sure it hasn't come from the other one. Since it is an increment, if the variable gets updated twice because it isn't volatile, things will still work.