dsmiley commented on code in PR #1625:
URL: https://github.com/apache/solr/pull/1625#discussion_r1188562791
##########
solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java:
##########
@@ -221,110 +217,18 @@ static void fetchReplicaMetrics(
@Override
public void close() throws IOException {}
- // uses metrics API to get node information
- static class MetricsFetchingSnitch extends ImplicitSnitch {
- @Override
- protected void getRemoteInfo(String solrNode, Set<String> requestedTags,
SnitchContext ctx) {
- if (!((ClientSnitchCtx) ctx).isNodeAlive(solrNode)) return;
- ClientSnitchCtx snitchContext = (ClientSnitchCtx) ctx;
- Map<String, Set<Object>> metricsKeyVsTag = new HashMap<>();
- for (String tag : requestedTags) {
- if (tag.startsWith(SYSPROP)) {
- metricsKeyVsTag
- .computeIfAbsent(
- "solr.jvm:system.properties:" +
tag.substring(SYSPROP.length()),
- k -> new HashSet<>())
- .add(tag);
- } else if (tag.startsWith(METRICS_PREFIX)) {
- metricsKeyVsTag
- .computeIfAbsent(tag.substring(METRICS_PREFIX.length()), k ->
new HashSet<>())
- .add(tag);
- }
- }
- if (!metricsKeyVsTag.isEmpty()) {
- fetchReplicaMetrics(solrNode, snitchContext, metricsKeyVsTag);
- }
-
- Set<String> groups = new HashSet<>();
- List<String> prefixes = new ArrayList<>();
- if (requestedTags.contains(DISK)) {
- groups.add("solr.node");
- prefixes.add("CONTAINER.fs.usableSpace");
- }
- if (requestedTags.contains(Variable.TOTALDISK.tagName)) {
- groups.add("solr.node");
- prefixes.add("CONTAINER.fs.totalSpace");
- }
- if (requestedTags.contains(CORES)) {
- groups.add("solr.node");
- prefixes.add("CONTAINER.cores");
- }
- if (requestedTags.contains(SYSLOADAVG)) {
- groups.add("solr.jvm");
- prefixes.add("os.systemLoadAverage");
- }
- if (requestedTags.contains(HEAPUSAGE)) {
- groups.add("solr.jvm");
- prefixes.add("memory.heap.usage");
- }
- if (groups.isEmpty() || prefixes.isEmpty()) return;
-
- ModifiableSolrParams params = new ModifiableSolrParams();
- params.add("group", StrUtils.join(groups, ','));
- params.add("prefix", StrUtils.join(prefixes, ','));
-
- try {
- SimpleSolrResponse rsp =
- snitchContext.invokeWithRetry(solrNode, CommonParams.METRICS_PATH,
params);
- NamedList<?> metrics = (NamedList<?>) rsp.nl.get("metrics");
- if (metrics != null) {
- // metrics enabled
- if (requestedTags.contains(Variable.FREEDISK.tagName)) {
- Object n = Utils.getObjectByPath(metrics, true,
"solr.node/CONTAINER.fs.usableSpace");
- if (n != null)
- ctx.getTags().put(Variable.FREEDISK.tagName,
Variable.FREEDISK.convertVal(n));
- }
- if (requestedTags.contains(Variable.TOTALDISK.tagName)) {
- Object n = Utils.getObjectByPath(metrics, true,
"solr.node/CONTAINER.fs.totalSpace");
- if (n != null)
- ctx.getTags().put(Variable.TOTALDISK.tagName,
Variable.TOTALDISK.convertVal(n));
- }
- if (requestedTags.contains(CORES)) {
- NamedList<?> node = (NamedList<?>) metrics.get("solr.node");
- int count = 0;
- for (String leafCoreMetricName : new String[] {"lazy", "loaded",
"unloaded"}) {
- Number n = (Number) node.get("CONTAINER.cores." +
leafCoreMetricName);
- if (n != null) count += n.intValue();
- }
- ctx.getTags().put(CORES, count);
- }
- if (requestedTags.contains(SYSLOADAVG)) {
- Number n =
- (Number) Utils.getObjectByPath(metrics, true,
"solr.jvm/os.systemLoadAverage");
- if (n != null) ctx.getTags().put(SYSLOADAVG, n.doubleValue());
- }
- if (requestedTags.contains(HEAPUSAGE)) {
- Number n = (Number) Utils.getObjectByPath(metrics, true,
"solr.jvm/memory.heap.usage");
- if (n != null) ctx.getTags().put(HEAPUSAGE, n.doubleValue() *
100.0d);
- }
- }
- } catch (Exception e) {
- throw new SolrException(
- SolrException.ErrorCode.SERVER_ERROR, "Error getting remote info",
e);
- }
- }
- }
-
@Override
public String toString() {
return Utils.toJSONString(this);
}
- static class ClientSnitchCtx extends SnitchContext {
- private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ static class RemoteCallCtx {
Review Comment:
Can you please add at least one javadoc line explaining what this is for? I
don't get it. It seems to be the "Command" software pattern? Maybe call it
"RemoteCallCmd" if so.
##########
solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/NodeValueFetcher.java:
##########
@@ -0,0 +1,203 @@
+/*
+ * 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.solrj.impl;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import org.apache.solr.client.solrj.response.SimpleSolrResponse;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.StrUtils;
+import org.apache.solr.common.util.Utils;
+import org.apache.zookeeper.KeeperException;
+
+/**
+ * This class is responsible for fetching metrics and other attributes from a
given node in Solr
+ * cluster. This is a helper class that is used by {@link
SolrClientNodeStateProvider}
Review Comment:
Still looking for a lifecycle like bit of documentation. It has no state;
right? In that case, all methods should probably be static and we say this is
a "utility" like class and document it's not instantiated.
##########
solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/NodeValueFetcher.java:
##########
@@ -0,0 +1,203 @@
+/*
+ * 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.solrj.impl;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import org.apache.solr.client.solrj.response.SimpleSolrResponse;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.StrUtils;
+import org.apache.solr.common.util.Utils;
+import org.apache.zookeeper.KeeperException;
+
+/**
+ * This class is responsible for fetching metrics and other attributes from a
given node in Solr
+ * cluster. This is a helper class that is used by {@link
SolrClientNodeStateProvider}
+ */
+public class NodeValueFetcher {
+ // well known tags
+ public static final String NODE = "node";
+ public static final String PORT = "port";
+ public static final String HOST = "host";
+ public static final String CORES = "cores";
+ public static final String ROLE = "role";
+ public static final String NODEROLE = "nodeRole";
+ public static final String SYSPROP = "sysprop.";
+ public static final Set<String> tags =
+ Set.of(NODE, PORT, HOST, CORES, Tags.FREEDISK.tagName, ROLE,
Tags.HEAPUSAGE.tagName);
+ public static final Pattern hostAndPortPattern =
Pattern.compile("(?:https?://)?([^:]+):(\\d+)");
+ public static final String METRICS_PREFIX = "metrics:";
+
+ public enum Tags {
+ FREEDISK(
+ "freedisk", "solr.node", "CONTAINER.fs.usableSpace",
"solr.node/CONTAINER.fs.usableSpace"),
+ TOTALDISK(
+ "totaldisk", "solr.node", "CONTAINER.fs.totalSpace",
"solr.node/CONTAINER.fs.totalSpace"),
+ CORES("cores", "solr.node", "CONTAINER.cores", null) {
+ @Override
+ public Object extractResult(Object root) {
+ NamedList<?> node = (NamedList<?>) Utils.getObjectByPath(root, false,
"solr.node");
+ int count = 0;
+ for (String leafCoreMetricName : new String[] {"lazy", "loaded",
"unloaded"}) {
+ Number n = (Number) node.get("CONTAINER.cores." +
leafCoreMetricName);
+ if (n != null) count += n.intValue();
+ }
+ return count;
+ }
+ },
+ SYSLOADAVG("sysLoadAvg", "solr.jvm", "os.systemLoadAverage",
"solr.jvm/os.systemLoadAverage"),
+ HEAPUSAGE("heapUsage", "solr.jvm", "memory.heap.usage",
"solr.jvm/memory.heap.usage");
+ public final String group, prefix, tagName, path;
+
+ Tags(String name, String group, String prefix, String path) {
+ this.group = group;
+ this.prefix = prefix;
+ this.tagName = name;
+ this.path = path;
+ }
+
+ public Object extractResult(Object root) {
+ Object v = Utils.getObjectByPath(root, true, path);
+ return v == null ? null : convertVal(v);
+ }
+
+ public Object convertVal(Object val) {
+ if (val instanceof String) {
+ return Double.valueOf((String) val);
+ } else if (val instanceof Number) {
+ Number num = (Number) val;
+ return num.doubleValue();
+
+ } else {
+ throw new IllegalArgumentException("Unknown type : " + val);
+ }
+ }
+ }
+
+ protected void getRemoteInfo(
Review Comment:
is not subclassed; should be private
##########
solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java:
##########
@@ -43,28 +43,24 @@
import org.apache.solr.common.cloud.ClusterState;
import org.apache.solr.common.cloud.DocCollection;
import org.apache.solr.common.cloud.Replica;
-import org.apache.solr.common.cloud.rule.ImplicitSnitch;
-import org.apache.solr.common.cloud.rule.SnitchContext;
import org.apache.solr.common.params.CommonParams;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.common.util.Pair;
-import org.apache.solr.common.util.StrUtils;
import org.apache.solr.common.util.Utils;
import org.apache.zookeeper.KeeperException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/** The <em>real</em> {@link NodeStateProvider}, which communicates with Solr
via SolrJ. */
public class SolrClientNodeStateProvider implements NodeStateProvider,
MapWriter {
- public static final String METRICS_PREFIX = "metrics:";
private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
private final CloudLegacySolrClient solrClient;
protected final Map<String, Map<String, Map<String, List<Replica>>>>
nodeVsCollectionVsShardVsReplicaInfo = new HashMap<>();
- private Map<String, Object> snitchSession = new HashMap<>();
+ private Map<String, Object> session = new HashMap<>();
Review Comment:
Can you please add a comment illustrating why the keys & values look like?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
