Clarify NameResolver.Listener behavior/expectations when the servers remain the same

[ST-DDT]

Is your feature request related to a problem?

I have a (Spring) discovery service based NameResolver which sends regular updates/ticks to the application, however it does not contain details which service name might have changed. So I have to check them for updates frequently, even if only one is service updated.

Should I verify whether the server addresses remained the same before invoking onAddresses/ onResult? How expensive is calling the method with the same servers over and over again? Does it trigger new connections to be made?

Describe the solution you'd like

Javadocs that explain what the listener will/are supposed to do if they get (partially) the same addresses.

Either:

The listener will create a diff with any previous server list and prepare (close) connections for the new (old) servers 

Or:

`NameResolver`s are discouraged from calling this method if the result hasn't changed from before.

Additional context

Ref #1770
Related #6523

[ST-DDT]

Also it would be nice to know (document/specify) whether setting new addresses/a new result on the listener will cause automatic connection attempts to those addresses or if they will only be used on demand.

[dapengzhang0]

It's unspecified, so it is up to the particular loadbalancer's implementation.

[ST-DDT]

So can I interpret this as "avoid unnecessary calls to the Listener#onResult() method"?

[dapengzhang0]

The name resolver implementation should avoid unnecessary calls to Listener#onResult() if nothing is changed from resolver's perspective. The name resolver should not assume that the listener implementation will do a refresh connection and should not try to achieve that purpose by calling Listener#onResult() multiple times with exactly the same result.

[ST-DDT]

If you could add that to the javadocs of the class/method that would be perfect.

[ST-DDT]

Maybe there should be a OnlyChangeForwardingListener in the API that only calls the underlying Listener if the result changed from previous calls. That way we don't have to implement the feature every time and you could also use it in the internal implementations such as DNSNameResolver. Although I would prefer it, if grpc-java would explicitly wrap the listener in it before passing it to the NameResolver.

[fangxlmr]

I noticed comments saying that implementations of NameResovler don't need to be thread-safe, but no doc here clarifying the listener/listerner2 interface is (should or should not) be thread-safe. I'm glad to have this in doc as well.

[ST-DDT]

IIRC the listener should be updated using the sync context. Which might indicate that it is not thread safe.

[fangxlmr]

IIRC the listener should be updated using the sync context. Which might indicate that it is not thread safe.

As stated in Javadoc,

Implementations don't need to be thread-safe. All methods are guaranteed to be called sequentially. Additionally, all methods that have side-effects, i.e., start(Listener2), shutdown() and refresh() are called from the same SynchronizationContext as returned by NameResolver.Args.getSynchronizationContext().

methods in NameResolver will be invoked in sync context whereas no more details on Listener itself. I think the derived statement, "Listener is not thread-safe", is more or less not convictive.