SolverRemote: Exclude remote servers with open circuits. #526 (#531)

* release notes

* removing old behavior test and activating the new one

* refactoring and testing

* refactoring tests

* refactoring

* [Gradle Release Plugin] - new version commit:  '3.25.7-snapshot'.

* disabling feature

* release notes

Author: mageddo

RELEASE-NOTES.md

@@ -1,3 +1,6 @@
+## 3.25.7
+* Refactoring SolverRemote module to implement circuit optimization #526
+
 ## 3.25.6
 * Refactoring SolverRemote module to implement circuit optimization #526
 

gradle.properties

@@ -1 +1 @@
-version=3.25.6-snapshot
+version=3.25.7-snapshot

src/main/java/com/mageddo/dnsproxyserver/solver/RemoteResolvers.java

@@ -1,41 +1,28 @@
 package com.mageddo.dnsproxyserver.solver;
 
 import com.mageddo.net.IpAddr;
-import com.mageddo.utils.Executors;
 
 import java.util.List;
-import java.util.concurrent.Executor;
-import java.util.concurrent.ExecutorService;
 import java.util.function.Function;
 
-public class RemoteResolvers implements AutoCloseable {
+public class RemoteResolvers {
 
   private final List<Resolver> resolvers;
-  private final ExecutorService executor;
 
-  private RemoteResolvers(List<Resolver> resolvers, ExecutorService executor) {
+  private RemoteResolvers(List<Resolver> resolvers) {
     this.resolvers = resolvers;
-    this.executor = executor;
   }
 
   public static RemoteResolvers of(List<IpAddr> servers, final Function<IpAddr, Resolver> resolverProvider) {
     final var resolvers = servers
       .stream()
       .map(resolverProvider)
       .toList();
-    return new RemoteResolvers(resolvers, Executors.newThreadExecutor());
+    return new RemoteResolvers(resolvers);
   }
 
   public List<Resolver> resolvers() {
     return this.resolvers;
   }
 
-  @Override
-  public void close() throws Exception {
-    this.executor.close();
-  }
-
-  public Executor getExecutor() {
-    return this.executor;
-  }
 }

src/main/java/com/mageddo/dnsproxyserver/solver/SolverRemote.java

@@ -2,10 +2,12 @@
 
 import com.mageddo.commons.circuitbreaker.CircuitCheckException;
 import com.mageddo.dns.utils.Messages;
-import com.mageddo.dnsproxyserver.solver.remote.application.CircuitBreakerService;
 import com.mageddo.dnsproxyserver.solver.remote.Request;
 import com.mageddo.dnsproxyserver.solver.remote.Result;
+import com.mageddo.dnsproxyserver.solver.remote.application.CircuitBreakerService;
+import com.mageddo.dnsproxyserver.solver.remote.application.ResolverStatsFactory;
 import com.mageddo.net.NetExecutorWatchdog;
+import com.mageddo.utils.Executors;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.ClassUtils;
@@ -20,6 +22,7 @@
 import java.util.Objects;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Stream;
 
@@ -33,9 +36,10 @@ public class SolverRemote implements Solver, AutoCloseable {
   static final String QUERY_TIMED_OUT_MSG = "Query timed out";
   public static final int PING_TIMEOUT_IN_MS = 1_500;
 
-  private final RemoteResolvers delegate;
-  private final NetExecutorWatchdog netWatchdog = new NetExecutorWatchdog();
   private final CircuitBreakerService circuitBreakerService;
+  private final ResolverStatsFactory resolverStatsFactory;
+  private final NetExecutorWatchdog netWatchdog = new NetExecutorWatchdog();
+  private final ExecutorService executor = Executors.newThreadExecutor();
 
   @Override
   public Response handle(Message query) {
@@ -54,8 +58,7 @@ public Response handle(Message query) {
 
   Result queryResultFromAvailableResolvers(Message query, StopWatch stopWatch) {
     final var lastErrorMsg = new AtomicReference<Message>();
-    // fixme #526 better to exclude open circuits.
-    final var resolvers = this.findResolversWithNonOpenCircuit();
+    final var resolvers = this.findResolversToUse();
     for (int i = 0; i < resolvers.size(); i++) {
 
       final var resolver = resolvers.get(i);
@@ -73,8 +76,8 @@ Result queryResultFromAvailableResolvers(Message query, StopWatch stopWatch) {
     return Result.fromErrorMessage(lastErrorMsg.get());
   }
 
-  List<Resolver> findResolversWithNonOpenCircuit() {
-    return this.delegate.resolvers();
+  List<Resolver> findResolversToUse() {
+    return this.resolverStatsFactory.findResolversWithNonOpenCircuit();
   }
 
   Request buildRequest(Message query, int resolverIndex, StopWatch stopWatch, Resolver resolver) {
@@ -93,13 +96,17 @@ Result safeQueryResult(Request req) {
   }
 
   Result queryResult(Request req) {
-    final var resFuture = req.sendQueryAsyncToResolver(this.delegate.getExecutor());
+    final var resFuture = this.sendQueryAsyncToResolver(req);
     if (this.isPingWhileGettingQueryResponseActive()) {
       this.pingWhileGettingQueryResponse(req, resFuture);
     }
     return this.transformToResult(resFuture, req);
   }
 
+  CompletableFuture<Message> sendQueryAsyncToResolver(Request req) {
+    return req.sendQueryAsyncToResolver(this.executor);
+  }
+
   void pingWhileGettingQueryResponse(Request req, CompletableFuture<Message> resFuture) {
     this.netWatchdog.watch(req.getResolverAddr(), resFuture, PING_TIMEOUT_IN_MS);
   }

src/main/java/com/mageddo/dnsproxyserver/solver/remote/ResolverStats.java

@@ -0,0 +1,17 @@
+package com.mageddo.dnsproxyserver.solver.remote;
+
+import com.mageddo.dnsproxyserver.solver.Resolver;
+import lombok.Builder;
+import lombok.Value;
+
+@Value
+@Builder
+public class ResolverStats {
+
+  private Resolver resolver;
+  private CircuitStatus circuitStatus;
+
+  public boolean isValidToUse() {
+    return !CircuitStatus.OPEN.equals(this.circuitStatus);
+  }
+}

src/main/java/com/mageddo/dnsproxyserver/solver/remote/application/ResolverStatsFactory.java

@@ -0,0 +1,38 @@
+package com.mageddo.dnsproxyserver.solver.remote.application;
+
+import com.mageddo.dnsproxyserver.solver.RemoteResolvers;
+import com.mageddo.dnsproxyserver.solver.Resolver;
+import com.mageddo.dnsproxyserver.solver.remote.ResolverStats;
+import com.mageddo.dnsproxyserver.solver.remote.mapper.ResolverMapper;
+import lombok.RequiredArgsConstructor;
+
+import javax.inject.Inject;
+import javax.inject.Singleton;
+import java.util.List;
+
+@Singleton
+@RequiredArgsConstructor(onConstructor = @__({@Inject}))
+public class ResolverStatsFactory {
+
+  private final CircuitBreakerService circuitBreakerService;
+  private final RemoteResolvers remoteResolvers;
+
+  public List<ResolverStats> find() {
+    return this.remoteResolvers.resolvers()
+      .stream()
+      .map(this::find)
+      .toList();
+  }
+
+  public List<Resolver> findResolversWithNonOpenCircuit() {
+    return this.find()
+      .stream()
+      .map(ResolverStats::getResolver)
+      .toList()
+      ;
+  }
+
+  public ResolverStats find(Resolver resolver) {
+    return ResolverMapper.toResolverStats(resolver, this.circuitBreakerService.getCircuitStatus(resolver.getAddress()));
+  }
+}

src/main/java/com/mageddo/dnsproxyserver/solver/remote/mapper/ResolverMapper.java

@@ -2,8 +2,11 @@
 
 import com.mageddo.dnsproxyserver.solver.Resolver;
 import com.mageddo.dnsproxyserver.solver.SimpleResolver;
+import com.mageddo.dnsproxyserver.solver.remote.CircuitStatus;
+import com.mageddo.dnsproxyserver.solver.remote.ResolverStats;
 import com.mageddo.dnsproxyserver.utils.InetAddresses;
 import com.mageddo.net.IpAddr;
+import org.apache.commons.compress.harmony.unpack200.bytecode.BCIRenumberedAttribute;
 
 import java.net.InetSocketAddress;
 import java.time.Duration;
@@ -21,4 +24,13 @@ public static Resolver from(InetSocketAddress addr) {
     resolver.setTimeout(DEFAULT_TIMEOUT);
     return resolver;
   }
+
+  public static ResolverStats toResolverStats(Resolver resolver, CircuitStatus status) {
+    return ResolverStats
+      .builder()
+      .resolver(resolver)
+      .circuitStatus(status)
+      .build()
+      ;
+  }
 }