[
https://issues.apache.org/jira/browse/ARTEMIS-4384?focusedWorklogId=875765&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-875765
]
ASF GitHub Bot logged work on ARTEMIS-4384:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 11/Aug/23 11:31
Start Date: 11/Aug/23 11:31
Worklog Time Spent: 10m
Work Description: gemmellr commented on code in PR #4574:
URL: https://github.com/apache/activemq-artemis/pull/4574#discussion_r1288186772
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/Shell.java:
##########
@@ -52,9 +52,9 @@ public class Shell implements Runnable {
protected String password;
- private static String RED_UNICODE = "\u001B[31m";
- private static String YELLOW_UNICODE = "\u001B[33m";
- private static String CLEAR_UNICODE = "\u001B[0m";
+ public static String RED_UNICODE = "\u001B[31m";
+ public static String YELLOW_UNICODE = "\u001B[33m";
+ public static String CLEAR_UNICODE = "\u001B[0m";
Review Comment:
These should perhaps be moved somewhere else to share them, if they are
being used from 'non Shell' places this doesnt seem like the natural place for
them anymore.
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/cluster/Verify.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * 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.activemq.artemis.cli.commands.tools.cluster;
+
+import org.apache.activemq.artemis.cli.commands.ActionContext;
+import org.apache.activemq.artemis.cli.commands.messages.ConnectionAbstract;
+import picocli.CommandLine;
+
[email protected](name = "verify", description = "Verify if all the nodes
match the same topology.")
+public class Verify extends ConnectionAbstract {
+
+ @CommandLine.Option(names = "--variance", description = "Allowed variance
in time before condiered a failure. (default=1000)")
Review Comment:
typo, considered
Also, a user might wonder, what are the units?
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/queue/StatQueue.java:
##########
@@ -85,6 +87,9 @@ public enum OPERATION {
@Option(names = "--maxColumnSize", description = "The max width of data
column. Set to -1 for no limit. Default is 25.")
private int maxColumnSize = DEFAULT_MAX_COLUMN_SIZE;
+ @Option(names = "--clustered", description = "Print statics for all the
nodes on the topology")
Review Comment:
stats?
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlTest.java:
##########
@@ -257,6 +257,13 @@ public void testSecurityCacheSizes() throws Exception {
Wait.assertEquals(usingCore() ? 8 : 1, () ->
serverControl.getAuthorizationCacheSize());
}
+ @Test
+ public void testCurrentTime() throws Exception {
+ long time = System.currentTimeMillis();
+ ActiveMQServerControl serverControl = createManagementControl();
+ Assert.assertTrue(serverControl.getCurrentTimeMillis() >= time);
Review Comment:
Printing the value on failure would be good otherwise theres little
indication what it might have been. Could also check the value was within some
delta rather than just >=.
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/cluster/ClusterVerifier.java:
##########
@@ -0,0 +1,321 @@
+/*
+ * 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.activemq.artemis.cli.commands.tools.cluster;
+
+import java.text.SimpleDateFormat;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Consumer;
+
+import org.apache.activemq.artemis.api.core.management.SimpleManagement;
+import org.apache.activemq.artemis.cli.commands.ActionContext;
+import org.apache.activemq.artemis.json.JsonArray;
+import org.apache.activemq.artemis.json.JsonObject;
+import org.apache.activemq.artemis.json.JsonString;
+
+public class ClusterVerifier implements AutoCloseable {
+
+ final String uri, user, password;
+
+ final SimpleManagement simpleManagement;
+
+ final long allowedVariance;
+
+ public ClusterVerifier(String uri, String user, String password) {
+ this(uri, user, password, 1000);
+ }
+
+ public ClusterVerifier(String uri, String user, String password, long
variance) {
+ this.uri = uri;
+ this.user = user;
+ this.password = password;
+ this.allowedVariance = variance;
+ this.simpleManagement = new SimpleManagement(uri, user, password);
+ }
+
+ @Override
+ public void close() throws Exception {
+ simpleManagement.close();
+ }
+
+ public ClusterVerifier open() throws Exception {
+ simpleManagement.open();
+ return this;
+ }
+
+ public boolean verify(ActionContext context) throws Exception {
+ String mainID = getNodeID();
+ JsonArray mainToplogy = fetchMainTopology();
+
+ AtomicBoolean verificationResult = new AtomicBoolean(true);
+
+ Map<String, TopologyItem> mainTopology = parseTopology(mainToplogy);
+ boolean supportTime = true;
+ try {
+ fetchTopologyTime(mainTopology);
+ } catch (Exception e) {
+ supportTime = false;
+ }
+
+ if (supportTime) {
+ verifyTime(context, mainTopology, verificationResult, supportTime);
+ } else {
+
context.out.println("*******************************************************************************************************************************");
+ context.out.println("Topology on " + uri + " nodeID=" + mainID + "
with " + mainToplogy.size() + " nodes :");
+ printTopology(context, "", mainToplogy);
+
context.out.println("*******************************************************************************************************************************");
+ }
+
+ mainTopology.forEach((a, b) -> {
+ try {
+ context.out.println("--> Verifying Topology for NodeID " +
b.nodeID + ", live = " + b.live + ", backup = " + b.backup);
+ if (b.live != null) {
+ context.out.println(" verification on live " + b.live);
+ if (!subVerify(context, b.live, mainTopology)) {
+ verificationResult.set(false);
+ } else {
+ context.out.println(" ok!");
+ }
+ }
+ } catch (Exception e) {
+ e.printStackTrace(context.out);
+ verificationResult.set(false);
+ }
+ });
+
+ return verificationResult.get();
+ }
+
+ protected void verifyTime(ActionContext context,
+ Map<String, TopologyItem> mainTopology,
+ AtomicBoolean verificationResult,
+ boolean supportTime) {
+
context.out.println("*******************************************************************************************************************************");
+
+ if (supportTime) {
+ Long[] times = fetchTopologyTime(mainTopology);
+
+ context.out.printf("%-40s | %-25s | %-19s | %-25s", "nodeID", "live",
"live local time", "backup");
+ context.out.println();
+ SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
+
+ long initialTime = System.currentTimeMillis();
+
+ mainTopology.forEach((id, node) -> {
+ context.out.printf("%-40s | %-25s | %-19s | %-25s", id, node.live,
formatDate(sdf, node.liveTime), node.backup);
+ context.out.println();
+ });
+
+ // how long it took to fetch the times. I'm adding this to the
allowed variance.
+ long latencyTime = System.currentTimeMillis() - initialTime;
+
+ long min = Long.MAX_VALUE, max = Long.MIN_VALUE;
+
+ for (long l : times) {
+
+ if (l < min) {
+ min = l;
+ }
+
+ if (l > max) {
+ max = l;
+ }
+ }
+
+ long variance = times.length > 0 ? (max - min) : 0;
+
+ long allowedVarianceWithLatency = allowedVariance + latencyTime;
+
+ if (variance < allowedVarianceWithLatency) {
+ context.out.println("Time variance in the cluster is " + variance
+ " milliseconds");
+ } else {
+ context.out.println("WARNING: Time variance in the cluster is
greater than " + allowedVarianceWithLatency + " milliseconds: " + variance + ".
Please verify your server's NTP configuration.");
+ verificationResult.set(false);
+ }
+ } else {
+ context.out.println("The current management version does not support
the getCurrentTimeMillis() method. Please verify whether your server's times
are in sync and whether they are using NTP.");
+ }
+
context.out.println("*******************************************************************************************************************************");
+ }
+
+ String formatDate(SimpleDateFormat sdf, long time) {
+ if (time == 0) {
+ return "";
+ } else {
+ return sdf.format(new Date(time));
+ }
+ }
+
+ protected Long[] fetchTopologyTime(Map<String, TopologyItem>
topologyItemMap) {
+ ArrayList<Long> times = new ArrayList<>(topologyItemMap.size() * 2);
+ topologyItemMap.forEach((id, node) -> {
+ if (node.live != null) {
+ try {
+ node.liveTime = fetchTime(node.live);
+ times.add(node.liveTime);
+ } catch (Exception e) {
+ ActionContext.system().err.println("Cannot fetch liveTime for
nodeID=" + id + ", url=" + node.live + " -> " + e.getMessage());
+ node.liveTime = 0;
+ }
+ }
+ });
+
+ return times.toArray(new Long[times.size()]);
+ }
+
+ private boolean subVerify(ActionContext context,
+ String uri,
+ Map<String, TopologyItem> mainTopology) throws
Exception {
+ JsonArray verifyTopology = fetchTopology(uri);
+ Map<String, TopologyItem> verifyTopologyMap =
parseTopology(verifyTopology);
+ String result = compareTopology(mainTopology, verifyTopologyMap);
+ if (result != null) {
+ context.out.println(result);
+ context.out.println(" Topology detailing for " + uri);
+ printTopology(context, " ", verifyTopology);
+ return false;
+ } else {
+ return true;
+ }
+ }
+
+ public String compareTopology(Map<String, TopologyItem> mainTopology,
Map<String, TopologyItem> compareTopology) {
+ if (mainTopology.size() != compareTopology.size()) {
+ return "main topology size " + mainTopology.size() + "!=
compareTopology size " + compareTopology.size();
+ }
+
+ int matchElements = 0;
+
+ for (Map.Entry<String, TopologyItem> entry : mainTopology.entrySet()) {
+ TopologyItem item = compareTopology.get(entry.getKey());
+ if (!item.equals(entry.getValue())) {
+ return "Topology mistmatch on " + item;
+ } else {
+ matchElements++;
+ }
+ }
+
+ if (matchElements != mainTopology.size()) {
+ return "Not all elements match!";
+ }
+
+ return null;
+
+ }
+
+ Map<String, TopologyItem> parseTopology(JsonArray topology) {
+ Map<String, TopologyItem> map = new LinkedHashMap<>();
+ navigateTopology(topology, t -> map.put(t.nodeID, t));
+ return map;
+ }
+
+ private void printTopology(ActionContext context, String prefix, JsonArray
topology) {
+ context.out.printf(prefix + "%-40s | %-25s | %-25s", "nodeID", "live",
"backup");
Review Comment:
Seems like you could reuse the format string for the heading and the actual
print below, would perhaps make it more obvious and save a concat.
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/ConnectionAbstract.java:
##########
@@ -233,6 +235,9 @@ void saveConnectionInfo(String brokerURL, String user,
String password) {
if (Shell.inShell() && CONNECTION_INFORMATION.get() == null) {
CONNECTION_INFORMATION.set(new ConnectionInformation(brokerURL, user,
password));
System.out.println("CLI connected to broker " + brokerURL + ", user:"
+ user);
+ this.brokerURL = brokerURL;
+ this.user = user;
+ this.password = password;
Review Comment:
Is this needed? Doesnt the recover method always reset them from the
CONNECTION_INFORMATION that we just created+set? If so isnt doing this
redundant?
Issue Time Tracking
-------------------
Worklog Id: (was: 875765)
Time Spent: 20m (was: 10m)
> CLI method to verify topology on all the nodes (cluster verify)
> ---------------------------------------------------------------
>
> Key: ARTEMIS-4384
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4384
> Project: ActiveMQ Artemis
> Issue Type: New Feature
> Affects Versions: 2.30.0
> Reporter: Clebert Suconic
> Assignee: Clebert Suconic
> Priority: Major
> Fix For: 2.31.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> This CLI method will download the topology of a server, and connect to all
> the individual nodes to verify their topology.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
