Some minor cleanups

Author: bryce-anderson

servicetalk-dns-discovery-netty/src/main/java/io/servicetalk/dns/discovery/netty/DefaultDnsClient.java

@@ -57,8 +57,8 @@
 import io.netty.util.ReferenceCountUtil;
 import io.netty.util.concurrent.Future;
 import io.netty.util.concurrent.FutureListener;
-import io.netty.util.concurrent.GenericFutureListener;
 import io.netty.util.concurrent.Promise;
+import io.netty.util.concurrent.PromiseNotifier;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -187,74 +187,69 @@ public long queryTimeoutMillis() {
 
     private static final class BackupRequestResolver implements DnsNameResolverDelegate {
 
-        private final DnsNameResolver primary;
-        private final DnsNameResolver backup;
+        private final DnsNameResolver primaryResolver;
+        private final DnsNameResolver backupResolver;
         private final EventLoop eventLoop;
 
         // TODO: we'll want to make sure we share the cache, make our backup request interval more flexible, and also
         //  give ourselves some sort of budget so we don't overload DNS, but these are all possible.
         BackupRequestResolver(DnsNameResolverBuilder builder, EventLoop eventLoop) {
-            primary = builder.build();
-            backup = builder.consolidateCacheSize(0).build();
+            primaryResolver = builder.build();
+            backupResolver = builder.consolidateCacheSize(0).build();
             this.eventLoop = eventLoop;
         }
 
         @Override
         public void close() {
             try {
-                primary.close();
+                primaryResolver.close();
             } finally {
-                backup.close();
+                backupResolver.close();
             }
         }
 
         @Override
         public Future<List<InetAddress>> resolveAll(String name) {
-            return compose(resolver -> resolver.resolveAll(name));
+            return withBackup(resolver -> resolver.resolveAll(name));
         }
 
         @Override
         public Future<List<DnsRecord>> resolveAll(DnsQuestion name) {
-            return compose(resolver -> resolver.resolveAll(name));
+            return withBackup(resolver -> resolver.resolveAll(name));
         }
 
         @Override
         public long queryTimeoutMillis() {
-            return primary.queryTimeoutMillis();
+            return primaryResolver.queryTimeoutMillis();
         }
 
-        private <T> Future<T> compose(Function<DnsNameResolver, Future<T>> query) {
+        private <T> Future<T> withBackup(Function<? super DnsNameResolver, ? extends Future<T>> query) {
+            Future<T> primaryQuery = query.apply(primaryResolver);
+            if (primaryQuery.isDone()) {
+                return primaryQuery;
+            }
+            int backupDelay = backupDelayMs();
+            if (backupDelay <= 0) {
+                // no backup for this request
+                return primaryQuery;
+            }
             Promise<T> result = eventLoop.newPromise();
-            Future<T> r1 = query.apply(primary);
-            Future<?> timer = eventLoop.schedule(() -> join(query.apply(backup), result, null),
-                    50, MILLISECONDS);
-            join(r1, result, timer);
+            Future<?> timer = eventLoop.schedule(() -> {
+                    if (allowBackupRequest()) {
+                        PromiseNotifier.cascade(false, query.apply(backupResolver), result);
+                    }
+                }, backupDelay, MILLISECONDS);
+            primaryQuery.addListener(_unused -> timer.cancel(true));
+            PromiseNotifier.cascade(false, primaryQuery, result);
             return result;
         }
 
-        private static <T> void join(Future<? extends T> future, Promise<T> promise, @Nullable Future<?> timer) {
-            // transfer results from future to promise and cancellation from promise to future.
-            future.addListener(new GenericFutureListener<Future<T>>() {
-                @Override
-                public void operationComplete(Future<T> future) {
-                    if (timer != null) {
-                        timer.cancel(true);
-                    }
-                    if (future.isSuccess()) {
-                        promise.trySuccess(future.getNow());
-                    } else if (!future.isCancelled()) {
-                        promise.tryFailure(future.cause());
-                    }
-                }
-            });
-            promise.addListener(new GenericFutureListener<Future<T>>() {
-                @Override
-                public void operationComplete(Future<T> promise) {
-                    if (promise.isCancelled()) {
-                        future.cancel(true);
-                    }
-                }
-            });
+        private boolean allowBackupRequest() {
+            return true;
+        }
+
+        private int backupDelayMs() {
+            return 50;
         }
     }