NMS-15776: WIP

OpenNMS · Jul 17, 2023 · 6617ae0 · 6617ae0
1 parent ffb4e18
commit 6617ae0
Show file tree

Hide file tree

Showing 13 changed files with 37 additions and 73 deletions.
diff --git a/core/health/api/src/main/java/org/opennms/core/health/api/DefaultPassiveHealthCheck.java b/core/health/api/src/main/java/org/opennms/core/health/api/DefaultPassiveHealthCheck.java
@@ -60,7 +60,7 @@ public DefaultPassiveHealthCheck(
 
     @Override
     public synchronized void setResponse(Response response) {
-        LOG.warn("Cache response - healthCheck: {}; status: {}; msg: {}", description, response.getStatus(), response.getMessage());
+        LOG.debug("Cache response - healthCheck: {}; status: {}; msg: {}", description, response.getStatus(), response.getMessage());
         cachedResponse = response;
         cachedResponseTimestamp = Instant.now();
     }

diff --git a/...health/impl/src/main/java/org/opennms/core/health/impl/ContainerIntegrityHealthCheck.java b/...health/impl/src/main/java/org/opennms/core/health/impl/ContainerIntegrityHealthCheck.java
@@ -116,26 +116,26 @@ public Response perform(Context context) {
                     if ((b.adapt(BundleRevision.class).getTypes() & BundleRevision.TYPE_FRAGMENT) != 0) {
                         break;
                     }
-                    health.add(this, new Response(Failure, "Bundle " + b.getBundleId() + " (" + b.getSymbolicName() + ") is resolved, but not active"));
+                    health.add(this, new Response(Failure, "Bundle " + b.getBundleId() + " is resolved, but not active"));
                     break;
                 // Waiting for dependencies
                 case Waiting:
                 case GracePeriod:
-                    health.add(this, new Response(Starting, "Bundle " + b.getBundleId() + " (" + b.getSymbolicName() + ") is waiting for dependencies"));
+                    health.add(this, new Response(Starting, "Bundle " + b.getBundleId() + " is waiting for dependencies"));
                     break;
                 // Installed, but not yet started
                 case Installed:
-                    health.add(this, new Response(Starting, "Bundle " + b.getBundleId() + " (" + b.getSymbolicName() + ") is not yet started"));
+                    health.add(this, new Response(Starting, "Bundle " + b.getBundleId() + " is not yet started"));
                     break;
                 // Starting
                 case Starting:
-                    health.add(this, new Response(Starting, "Bundle " + b.getBundleId() + " (" + b.getSymbolicName() + ") is starting"));
+                    health.add(this, new Response(Starting, "Bundle " + b.getBundleId() + " is starting"));
                     break;
                 // Stopping, Failed ur Unknown are considered Failures
                 case Stopping:
                 case Failure:
                 case Unknown:
-                    health.add(this, new Response(Failure, "Bundle " + b.getBundleId() + " (" + b.getSymbolicName() + ") is not started"));
+                    health.add(this, new Response(Failure, "Bundle " + b.getBundleId() + " is not started"));
                     break;
             }
         }

diff --git a/core/health/impl/src/main/java/org/opennms/core/health/impl/DefaultHealthCheckService.java b/core/health/impl/src/main/java/org/opennms/core/health/impl/DefaultHealthCheckService.java
@@ -146,14 +146,6 @@ private CompletionStage<Pair<HealthCheck, Response>> completionStage(HealthCheck
     private CompletionStage<Health> runChecks(Context context, List<HealthCheck> checks, ProgressListener listener) {
         return FutureUtils.traverse(checks, check -> completionStage(check, context, listener)).thenApply(list -> {
             var health = new Health(list);
-            for(var pair : list) {
-                if (pair.getRight().isSuccess()) {
-                    LOG.warn("BMRHGA: " + pair.getLeft().getDescription() + ": Success");
-                } else {
-                    LOG.warn("BMRHGA: " + pair.getLeft().getDescription() + ": Failure (msg = '" + pair.getRight().getMessage() + "')");
-                }
-            }
-
             if (listener != null) {
                 listener.onAllHealthChecksCompleted(health);
             }

diff --git a/.../rest/src/main/java/org/opennms/core/health/rest/internal/HealthCheckRestServiceImpl.java b/.../rest/src/main/java/org/opennms/core/health/rest/internal/HealthCheckRestServiceImpl.java
@@ -43,10 +43,10 @@
 import org.opennms.core.health.api.Health;
 import org.opennms.core.health.api.HealthCheckService;
 import org.opennms.core.health.rest.HealthCheckRestService;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import io.vavr.control.Either;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class HealthCheckRestServiceImpl implements HealthCheckRestService {
 

diff --git a/docs/modules/operation/pages/deep-dive/provisioning/scalability.adoc b/docs/modules/operation/pages/deep-dive/provisioning/scalability.adoc
@@ -93,23 +93,7 @@ You can also disable this scan by setting `scan-interval` to `-1`.
 
 During the provisioning process hostnames are determined for each interface IP address by DNS reverse lookups.
 A thread pool is used to allow concurrent DNS lookups.
-You can use the following properties to configure this thread pool:
-
-.Thread pool configuration parameters for DNS lookups
-[options="header"]
-[cols="1,3,2"]
-|===
-| Parameter
-| Description
-| Default value
-
-| org.opennms.netmgt.provision.dns.client.rpc.maxThreadPoolSize
-| Maximum thread pool size.
-| 64
-| org.opennms.netmgt.provision.dns.client.rpc.queueCapacity
-| Maximum queue size for pending DNS lookups.
-| 4096
-|===
+You can use the property `org.opennms.netmgt.provision.dns.client.rpc.threadCount` to configure the thread pool's fixed size (default 64).
 
 NOTE: To completely disable DNS reverse lookups, set the property `org.opennms.provisiond.reverseResolveRequisitionIpInterfaceHostnames` to `false`.
 
diff --git a/opennms-base-assembly/src/main/filtered/etc/opennms.properties b/opennms-base-assembly/src/main/filtered/etc/opennms.properties
@@ -869,9 +869,8 @@ org.opennms.newts.nan_on_counter_wrap=true
 # org.opennms.mate.maxRecursionDepth=8
 
 # ###### DNS tuning options ######
-# A thread pool is used to allow concurrent DNS lookups on OpenNMS instance.
-# org.opennms.netmgt.provision.dns.client.rpc.maxThreadPoolSize=64
-# org.opennms.netmgt.provision.dns.client.rpc.queueCapacity=4096
+# A thread pool is used to allow concurrent DNS lookups on OpenNMS Minions.
+# org.opennms.netmgt.provision.dns.client.rpc.threadCount=64
 
 # By default hostnames are determined for a node's IP addresses during the provisioning's audit phase.
 # org.opennms.provisiond.reverseResolveRequisitionIpInterfaceHostnames=true
diff --git a/opennms-provision/opennms-detectorclient-rpc/pom.xml b/opennms-provision/opennms-detectorclient-rpc/pom.xml
@@ -71,11 +71,6 @@
       <artifactId>dnsjava-dependencies</artifactId>
       <type>pom</type>
     </dependency>
-    <dependency>
-      <groupId>io.github.resilience4j</groupId>
-      <artifactId>resilience4j-bulkhead</artifactId>
-      <version>${resilience4jVersion}</version>
-    </dependency>
     <dependency>
       <groupId>org.opennms.core.ipc.rpc</groupId>
       <artifactId>org.opennms.core.ipc.rpc.jms-impl</artifactId>

diff --git a/...c/src/main/java/org/opennms/netmgt/provision/dns/client/rpc/DnsLookupClientRpcModule.java b/...c/src/main/java/org/opennms/netmgt/provision/dns/client/rpc/DnsLookupClientRpcModule.java
@@ -31,35 +31,29 @@
 import java.net.InetAddress;
 import java.net.UnknownHostException;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
 
 import org.opennms.core.rpc.xml.AbstractXmlRpcModule;
 import org.opennms.core.utils.InetAddressUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.xbill.DNS.Address;
 
-import io.github.resilience4j.bulkhead.ThreadPoolBulkhead;
-import io.github.resilience4j.bulkhead.ThreadPoolBulkheadConfig;
-
 public class DnsLookupClientRpcModule extends AbstractXmlRpcModule<DnsLookupRequestDTO, DnsLookupResponseDTO> {
 
     private static final Logger LOG = LoggerFactory.getLogger(DnsLookupClientRpcModule.class);
 
     public static final String RPC_MODULE_ID = "DNS";
 
-    private final ThreadPoolBulkhead threadPoolBulkhead;
+    private final ExecutorService executorService;
 
-    public DnsLookupClientRpcModule(final int maxThreadPoolSize, final int queueCapacity) {
+    public DnsLookupClientRpcModule(final int threadCount) {
         super(DnsLookupRequestDTO.class, DnsLookupResponseDTO.class);
 
-        final ThreadPoolBulkheadConfig threadPoolBulkheadConfig = ThreadPoolBulkheadConfig.custom()
-                .queueCapacity(queueCapacity)
-                .maxThreadPoolSize(maxThreadPoolSize)
-                .build();
-
-        this.threadPoolBulkhead = ThreadPoolBulkhead.of("dns-lookup-rpc-module", threadPoolBulkheadConfig);
+        this.executorService = Executors.newFixedThreadPool(threadCount);
 
-        LOG.debug("Configuring ThreadPoolBulkhead: maxThreadPoolSize={}, queueSize={}", maxThreadPoolSize, queueCapacity);
+        LOG.debug("Configuring fixed-sized ThreadPool using threadCount={}", threadCount);
     }
 
     @Override
@@ -74,7 +68,7 @@ public String getId() {
 
     @Override
     public CompletableFuture<DnsLookupResponseDTO> execute(DnsLookupRequestDTO request) {
-        return this.threadPoolBulkhead.submit(() -> {
+        return CompletableFuture.supplyAsync(() -> {
                 final InetAddress addr = InetAddressUtils.addr(request.getHostRequest());
                 final DnsLookupResponseDTO dto = new DnsLookupResponseDTO();
                 final QueryType queryType = request.getQueryType();
@@ -99,6 +93,6 @@ public CompletableFuture<DnsLookupResponseDTO> execute(DnsLookupRequestDTO reque
                     dto.setHostResponse(hostName);
                 }
                 return dto;
-        }).toCompletableFuture();
+        }, this.executorService);
     }
 }
diff --git a/...nms-detectorclient-rpc/src/main/resources/META-INF/opennms/applicationContext-rpc-dns.xml b/...nms-detectorclient-rpc/src/main/resources/META-INF/opennms/applicationContext-rpc-dns.xml
@@ -12,8 +12,7 @@
 	<context:annotation-config />
 
     <bean id="dnsLookupClientRpcModule" class="org.opennms.netmgt.provision.dns.client.rpc.DnsLookupClientRpcModule" >
-        <constructor-arg value="${org.opennms.netmgt.provision.dns.client.rpc.maxThreadPoolSize:4096}"/>
-        <constructor-arg value="${org.opennms.netmgt.provision.dns.client.rpc.queueCapacity:64}"/>
+        <constructor-arg value="${org.opennms.netmgt.provision.dns.client.rpc.threadCount:64}"/>
     </bean>
 
     <bean id="locationAwareDnsClient" class="org.opennms.netmgt.provision.dns.client.rpc.LocationAwareDnsLookupClientRpcImpl" />

diff --git a/...-provision/opennms-detectorclient-rpc/src/main/resources/OSGI-INF/blueprint/blueprint.xml b/...-provision/opennms-detectorclient-rpc/src/main/resources/OSGI-INF/blueprint/blueprint.xml
@@ -13,8 +13,7 @@
 
     <cm:property-placeholder id="dnsLookupProperties" persistent-id="org.opennms.netmgt.provision.dns.client.rpc" update-strategy="reload">
         <cm:default-properties>
-            <cm:property name="queueCapacity" value="4096"/>
-            <cm:property name="maxThreadPoolSize" value="64"/>
+            <cm:property name="threadCount" value="64"/>
         </cm:default-properties>
     </cm:property-placeholder>
 
@@ -28,8 +27,7 @@
 
     <service interface="org.opennms.core.rpc.api.RpcModule">
         <bean class="org.opennms.netmgt.provision.dns.client.rpc.DnsLookupClientRpcModule">
-            <argument value="${maxThreadPoolSize}"/>
-            <argument value="${queueCapacity}"/>
+            <argument value="${threadCount}"/>
         </bean>
     </service>
 

diff --git a/...rovisiond/src/main/java/org/opennms/netmgt/provision/service/DefaultHostnameResolver.java b/...rovisiond/src/main/java/org/opennms/netmgt/provision/service/DefaultHostnameResolver.java
@@ -32,6 +32,7 @@
 import java.util.Objects;
 import java.util.concurrent.CompletableFuture;
 
+import org.opennms.core.utils.InetAddressUtils;
 import org.opennms.netmgt.provision.LocationAwareDnsLookupClient;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -49,6 +50,14 @@ public DefaultHostnameResolver(LocationAwareDnsLookupClient locationAwareDnsLook
     @Override
     public CompletableFuture<String> getHostnameAsync(final InetAddress addr, final String location) {
         LOG.debug("Performing reverse lookup on {} at location {}", addr, location);
-        return m_locationAwareDnsLookupClient.reverseLookup(addr, location);
+        return m_locationAwareDnsLookupClient.reverseLookup(addr, location).handle((result, e) -> {
+            if (e == null) {
+                LOG.debug("Reverse lookup returned {} for {} at location {}", result, addr, location);
+                return result;
+            } else {
+                LOG.warn("Reverse lookup failed for {} at location {}. Using IP address as hostname.", addr, location);
+                return InetAddressUtils.str(addr);
+            }
+        });
     }
 }
diff --git a/...ennms-provisiond/src/main/java/org/opennms/netmgt/provision/service/HostnameResolver.java b/...ennms-provisiond/src/main/java/org/opennms/netmgt/provision/service/HostnameResolver.java
@@ -32,21 +32,13 @@
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
 
-import org.opennms.core.utils.InetAddressUtils;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 public interface HostnameResolver {
-    Logger LOG = LoggerFactory.getLogger(HostnameResolver.class);
 
     default String getHostname(final InetAddress addr, final String location) {
         try {
-            final String result = getHostnameAsync(addr, location).get();
-            LOG.debug("Reverse lookup returned {} for {} at location {}", result, addr, location);
-            return result;
+            return getHostnameAsync(addr, location).get();
         } catch (InterruptedException | ExecutionException e) {
-            LOG.warn("Reverse lookup failed for {} at location {}. Using IP address as hostname.", addr, location);
-            return InetAddressUtils.str(addr);
+            throw new RuntimeException(e);
         }
     }
 

diff --git a/...-provisiond/src/main/java/org/opennms/netmgt/provision/service/RequisitionAccountant.java b/...-provisiond/src/main/java/org/opennms/netmgt/provision/service/RequisitionAccountant.java
@@ -134,7 +134,9 @@ int dnsLookupsTotal() {
     @Override
     public void completeModelImport(Requisition req) {
         LOG.debug("Waiting for {} scheduled DNS lookups, {} DNS lookups pending", dnsLookupsTotal(), dnsLookupsPending());
-        CompletableFuture.allOf(m_dnsLookups.toArray(CompletableFuture[]::new)).join();
+
+        m_dnsLookups.stream().map(CompletableFuture::join);
+
         LOG.debug("All {} scheduled DNS lookups completed", dnsLookupsTotal());
     }
 }