SOLR-15823: Broadcast V2 logging level changes to named nodes and migrate the Admin UI to V2

changelog/unreleased/SOLR-15823.yml

@@ -0,0 +1,7 @@
+title: V2 logging API can broadcast log level changes to multiple nodes, and the Admin UI logging screen now uses it.
+type: added
+authors:
+  - name: Nick Shanin
+links:
+  - name: SOLR-15823
+    url: https://issues.apache.org/jira/browse/SOLR-15823

solr/api/src/java/org/apache/solr/client/api/endpoint/NodeLoggingApis.java

@@ -42,8 +42,13 @@ public interface NodeLoggingApis {
   @Path("/levels")
   @Operation(
       summary = "Set one or more logger levels on the target node.",
+      description =
+          "If the 'nodes' parameter is provided, the level changes are instead applied to "
+              + "each of the named nodes (or to every live node, if 'nodes' is 'all'), and the "
+              + "response reports the per-node results.",
       tags = {"logging"})
-  LoggingResponse modifyLocalLogLevel(List<LogLevelChange> requestBody);
+  LoggingResponse modifyLocalLogLevel(
+      @QueryParam("nodes") String nodes, List<LogLevelChange> requestBody);
 
   @GET
   @Path("/messages")

solr/api/src/java/org/apache/solr/client/api/model/LoggingResponse.java

@@ -19,7 +19,7 @@
 import com.fasterxml.jackson.annotation.JsonProperty;
 
 /** Generic logging response that includes the name of the log watcher (e.g. "Log4j2") */
-public class LoggingResponse extends SolrJerseyResponse {
+public class LoggingResponse extends NodeProxyingJerseyResponse {
   @JsonProperty("watcher")
   public String watcherName;
 }

solr/api/src/java/org/apache/solr/client/api/model/NodeProxyingJerseyResponse.java

@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.api.model;
+
+import com.fasterxml.jackson.annotation.JsonAnyGetter;
+import com.fasterxml.jackson.annotation.JsonAnySetter;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * A {@link SolrJerseyResponse} for a node-local API that can also be proxied to other live nodes
+ * via the {@code nodes} request parameter (see {@code
+ * org.apache.solr.handler.admin.proxy.V2SolrRequestBasedProxy}).
+ *
+ * <p>Each proxied node's response is merged in under a top-level key equal to that node's name.
+ */
+public class NodeProxyingJerseyResponse extends SolrJerseyResponse {
+
+  // Object, not a response subtype, since @JsonAnySetter below also feeds this map raw values.
+  public Map<String, Object> remoteNodeData = new LinkedHashMap<>();
+
+  @JsonAnyGetter
+  public Map<String, Object> remoteNodeData() {
+    return remoteNodeData;
+  }
+
+  @JsonAnySetter
+  public void setRemoteNodeResponse(String field, Object value) {
+    remoteNodeData.put(field, value);
+  }
+
+  /**
+   * Nodes that were asked to apply a broadcast request (via the 'nodes' parameter) but did not
+   * return a response, e.g. because they timed out or errored (see {@code
+   * RemoteRequestProxy#getFailedNodes()}). Null for requests that were not broadcast, and empty
+   * when every requested node responded.
+   */
+  @JsonProperty("failedNodes")
+  public List<String> failedNodes;
+}

solr/api/src/java/org/apache/solr/client/api/model/NodeSystemResponse.java

@@ -16,30 +16,14 @@
  */
 package org.apache.solr.client.api.model;
 
-import com.fasterxml.jackson.annotation.JsonAnyGetter;
-import com.fasterxml.jackson.annotation.JsonAnySetter;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import java.util.Date;
-import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
 /** Response from /node/system */
-public class NodeSystemResponse extends SolrJerseyResponse {
-
-  // Object, not NodeSystemResponse, since @JsonAnySetter below also feeds this map raw values.
-  public Map<String, Object> remoteNodeData = new LinkedHashMap<>();
-
-  @JsonAnyGetter
-  public Map<String, Object> remoteNodeData() {
-    return remoteNodeData;
-  }
-
-  @JsonAnySetter
-  public void setRemoteNodeResponse(String field, Object value) {
-    remoteNodeData.put(field, value);
-  }
+public class NodeSystemResponse extends NodeProxyingJerseyResponse {
 
   @JsonProperty public String host;
   @JsonProperty public String node;

solr/core/src/java/org/apache/solr/handler/admin/LoggingHandler.java

@@ -75,7 +75,7 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw
     String[] set = params.getParams("set");
     if (set != null) {
       final List<LogLevelChange> changes = NodeLogging.parseLogLevelChanges(set);
-      squashV2Response(rsp, loggingApi.modifyLocalLogLevel(changes));
+      squashV2Response(rsp, loggingApi.modifyLocalLogLevel(null, changes));
     }
 
     String since = req.getParams().get("since");

solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeSystemInfo.java

@@ -80,7 +80,11 @@ public void processTypedProxiedResponse(
                 response.remoteNodeData.put(nodeName, proxiedResponse);
               }
             };
-        return reqProxy.proxyRequest();
+        boolean proxied = reqProxy.proxyRequest();
+        if (proxied) {
+          response.failedNodes = reqProxy.getFailedNodes();
+        }
+        return proxied;
       }
     } catch (Exception e) {
       throw new SolrException(

solr/core/src/java/org/apache/solr/handler/admin/api/NodeLogging.java

@@ -36,17 +36,20 @@
 import org.apache.solr.client.api.model.LogMessagesResponse;
 import org.apache.solr.client.api.model.LoggingResponse;
 import org.apache.solr.client.api.model.SetThresholdRequestBody;
+import org.apache.solr.client.solrj.request.LoggingApi;
 import org.apache.solr.common.SolrDocumentList;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.proxy.RemoteRequestProxy;
+import org.apache.solr.handler.admin.proxy.V2SolrRequestBasedProxy;
 import org.apache.solr.jersey.PermissionName;
 import org.apache.solr.logging.LogWatcher;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-// TODO Add support for 'nodes' param once SOLR-16738 is completed.
 /**
- * V2 APIs for getting or setting log levels on an individual node.
+ * V2 APIs for getting or setting log levels on an individual node, or for broadcasting log level
+ * changes across nodes.
  *
  * <p>These APIs ('/api/node/logging' and descendants) are analogous to the v1 /admin/info/logging.
  */
@@ -84,20 +87,63 @@ public ListLevelsResponse listAllLoggersAndLevels() {
 
   @Override
   @PermissionName(CONFIG_EDIT_PERM)
-  public LoggingResponse modifyLocalLogLevel(List<LogLevelChange> requestBody) {
+  public LoggingResponse modifyLocalLogLevel(String nodes, List<LogLevelChange> requestBody) {
     ensureLogWatcherEnabled();
     final LoggingResponse response = instantiateLoggingResponse(LoggingResponse.class);
 
     if (requestBody == null) {
       throw new SolrException(BAD_REQUEST, "Missing request body");
     }
 
+    if (nodes != null && !nodes.isEmpty()) {
+      proxyLevelChangesToNodes(response, nodes, requestBody);
+      return response;
+    }
+
     for (LogLevelChange change : requestBody) {
       watcher.setLogLevel(change.logger, change.level);
     }
     return response;
   }
 
+  /**
+   * Applies the given log level changes on other nodes, mirroring how {@link GetNodeSystemInfo}
+   * fans its request out. The receiving node does not apply the changes locally; it is covered only
+   * if the resolved node set includes it, in which case it calls itself over HTTP.
+   *
+   * <p>{@link RemoteRequestProxy#validateNodeNames} rejects an unknown node name before any request
+   * goes out, so a broadcast either reaches every named node or none of them; a node that stops
+   * responding after that point (timeout, error, or leaving the live set mid-request) is instead
+   * reported via {@link RemoteRequestProxy#getFailedNodes()}.
+   */
+  private void proxyLevelChangesToNodes(
+      LoggingResponse response, String nodes, List<LogLevelChange> requestBody) {
+    if (coreContainer == null || coreContainer.getZkController() == null) {
+      throw new SolrException(
+          BAD_REQUEST, "The 'nodes' parameter is only supported in SolrCloud mode");
+    }
+    try {
+      final var req = new LoggingApi.ModifyLocalLogLevel();
+      requestBody.forEach(req::addLogLevelChange);
+      req.setNodes(nodes);
+      final var reqProxy =
+          new V2SolrRequestBasedProxy<LoggingResponse>(coreContainer, req) {
+            @Override
+            public void processTypedProxiedResponse(
+                String nodeName, LoggingResponse proxiedResponse) {
+              response.remoteNodeData.put(nodeName, proxiedResponse);
+            }
+          };
+      reqProxy.proxyRequest();
+      response.failedNodes = reqProxy.getFailedNodes();
+    } catch (SolrException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new SolrException(
+          SolrException.ErrorCode.SERVER_ERROR, "Error occurred while proxying to other nodes", e);
+    }
+  }
+
   @Override
   @PermissionName(CONFIG_READ_PERM)
   public LogMessagesResponse fetchLocalLogMessages(Long boundingTimeMillis) {

solr/core/src/java/org/apache/solr/handler/admin/proxy/RemoteRequestProxy.java

@@ -20,12 +20,15 @@
 import java.io.IOException;
 import java.lang.invoke.MethodHandles;
 import java.net.URI;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.TreeSet;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
@@ -51,6 +54,9 @@ public abstract class RemoteRequestProxy {
 
   protected final CoreContainer coreContainer;
 
+  // Per-request state: each request builds a fresh proxy instance, so this is never shared.
+  private final Set<String> failedNodes = new TreeSet<>();
+
   public RemoteRequestProxy(CoreContainer container) {
     this.coreContainer = container;
   }
@@ -90,6 +96,15 @@ public boolean proxyRequest() {
     return true;
   }
 
+  /**
+   * Destination nodes (per {@link #getDestinationNodes()}) that did not produce a response, e.g.
+   * because the request errored or timed out. Only meaningful after {@link #proxyRequest()} returns
+   * true; empty if every destination node responded.
+   */
+  public List<String> getFailedNodes() {
+    return new ArrayList<>(failedNodes);
+  }
+
   private void bulkProcessResponses(Map<String, Future<NamedList<Object>>> responseFutures) {
     for (Map.Entry<String, Future<NamedList<Object>>> entry : responseFutures.entrySet()) {
       try {
@@ -98,8 +113,10 @@ private void bulkProcessResponses(Map<String, Future<NamedList<Object>>> respons
       } catch (ExecutionException | InterruptedException ee) {
         log.warn(
             "Exception when fetching result from node {}", entry.getKey(), ee.getCause()); // nowarn
+        failedNodes.add(entry.getKey());
       } catch (TimeoutException e) {
         log.warn("Timeout exceeded waiting for response from proxied node {}", entry.getKey());
+        failedNodes.add(entry.getKey());
       }
     }
     if (log.isDebugEnabled()) {
@@ -145,9 +162,15 @@ private CompletableFuture<NamedList<Object>> callRemoteNode(
   /**
    * Resolve node names from the "nodes" parameter into a set of live node names.
    *
+   * <p>For an explicit (non-"all") list, every named node is checked against the live-node set
+   * before returning, so a caller that validates up front (e.g. via {@link #getDestinationNodes()}
+   * before dispatching) fails atomically instead of partway through a broadcast: {@link
+   * #callRemoteNode} re-checks liveness per node regardless, but only once dispatch is already
+   * underway.
+   *
    * @param nodeNames the value of the "nodes" parameter ("all" or comma-separated node names)
    * @return set of resolved node names
-   * @throws SolrException if node format is invalid
+   * @throws SolrException if node format is invalid, or a named node is not live
    */
   protected Set<String> validateNodeNames(String nodeNames) {
     Set<String> liveNodes =
@@ -165,6 +188,13 @@ protected Set<String> validateNodeNames(String nodeNames) {
             SolrException.ErrorCode.BAD_REQUEST, "Parameter " + PARAM_NODES + " has wrong format");
       }
     }
+    final List<String> unknownNodes =
+        nodes.stream().filter(n -> !liveNodes.contains(n)).sorted().toList();
+    if (!unknownNodes.isEmpty()) {
+      throw new SolrException(
+          SolrException.ErrorCode.BAD_REQUEST,
+          "Requested nodes are not part of the cluster: " + unknownNodes);
+    }
     log.debug("Nodes requested: {}", nodes);
     return nodes;
   }

solr/core/src/test/org/apache/solr/handler/admin/api/GetNodeSystemInfoSolrCloudTest.java

@@ -44,5 +44,7 @@ public void testGetNodeInfoWithNodesParam() throws Exception {
     assertEquals(0, infoRsp.responseHeader.status);
     assertNotNull(infoRsp.remoteNodeData);
     assertTrue(infoRsp.remoteNodeData.containsKey(nodeName));
+    assertNotNull(infoRsp.failedNodes);
+    assertTrue(infoRsp.failedNodes.isEmpty());
   }
 }