Add support for polling SRV records and getting update notifications

[spotify]

Add support for polling SRV records and getting update notifications

[spkrka]

Nice initiative! Folsom uses dns-data in this way, see example here: https://github.com/spotify/folsom/blob/master/src/main/java/com/spotify/folsom/ketama/SrvKetamaClient.java

Should this new API perhaps also take a ScheduledExecutorService as input?
The API probably also needs a way to stop listening/polling for updates. I guess that would go into the PollingDnsSrvResolver object.

[rouzwawi]

@spkrka good points.

Providing a ScheduledExecutorService when creating the poller makes sense, will add that.

I think having a way to cancel individual lookups would be useful too. Actually the EndpointProvider interface has a close() method that somehow got lost in the description. And a close method on PollingDnsSrvResolver would then cancel all EndpointProviders created from that resolver.

On a side note; I'm not sure if I like the EndpointProvider name. Can you think of something more descriptive?

[spkrka]

I guess I'd like the API to look something like this:

public class PeriodicDnsSrvUpdater {
  public PeriodicDnsSrvUpdater(String fqdn, long period, TimeUnit unit, DnsSrvResolver resolver, ScheduledExecutorService service);

  // stop polling for updates
  public void close();

  public void addListener(DnsSrvUpdateCallback callback);
  public void removeListener(DnsSrvUpdateCallback callback);
}

public interface DnsSrvUpdateCallback {
  void endpointsChanged(Set<HostAndPort> newEndpoints, Set<HostAndPort> oldEndpoints);
}

[pettermahlen]

Like the discussion so far, a couple of points:

• I think HostAndPort is too restrictive, given that you might want to include priority and weight in there as well.
• I sort of think that doing conversion (and hence making the 'listener' take a type parameter) is potentially something best left outside this project. It wouldn't be hard or very noisy to write client code that simply applies a transform to the two Sets the listener receives, and I like the 'if in doubt, leave it out' approach.

Some thoughts about names:

• I think PeriodicDnsSrvUpdater and DnsSrvUpdateCallback are pretty descriptive, but also quite long.
• Another option could maybe be to take inspiration from Guava CacheBuilder, maybe something like:
  • DnsSrvChangeNotifier ('Periodic' is perhaps redundant/too specific in the interface)
  • ChangeListener (since this is nested in the DnsSrvChangeNotifier, we might not need 'DnsSrv')
  • onChange(ChangeNotification), where the ChangeNotification includes the two sets, and possibly some other information if we decide that is useful.

We could possibly drop 'Change' as well from the names.

We should probably write some client code for some of our use cases to see what it looks like.

[pettermahlen]

Re the type parameter, I just noticed that @rouzwawi was pointing out that including transformation in the API leads to more accurate notifications, since a change in a LookupResult might not be a change after the transformation. For instance, if somebody doesn't care about weight and priority and transforms to HostAndPort, then a change in weight for one of the results wouldn't lead to a notification. That I think motivates including transformation in the API.

[rouzwawi]

Updated API based on this thread and started implementation in #9

[rouzwawi]

Fixed by #9