From 636ab42bd5b2d2e55d37ce653daf23955f869a38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20de=20la=20Pe=C3=B1a?= Date: Fri, 10 Sep 2021 13:20:46 +0100 Subject: [PATCH] Fix missed wait latencies in the output of `nodetool tpstats -F` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit patch by Dimitar Dimitrov and Andrés de la Peña; reviewed by Ekaterina Dimitrova for CASSANDRA-16938 Co-authored-by: Andrés de la Peña Co-authored-by: Dimitar Dimitrov --- CHANGES.txt | 1 + .../tools/nodetool/stats/StatsPrinter.java | 17 +- .../tools/nodetool/stats/TpStatsHolder.java | 11 +- .../cassandra/tools/NodeToolTPStatsTest.java | 12 +- .../nodetool/stats/StatsPrinterTest.java | 186 ++++++++++++++++++ 5 files changed, 213 insertions(+), 14 deletions(-) create mode 100644 test/unit/org/apache/cassandra/tools/nodetool/stats/StatsPrinterTest.java diff --git a/CHANGES.txt b/CHANGES.txt index 1a487c4c51..b332be61ca 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.0.2 + * Fix missed wait latencies in the output of `nodetool tpstats -F` (CASSANDRA-16938) * Remove all the state pollution between tests in SSTableReaderTest (CASSANDRA-16888) * Delay auth setup until after gossip has settled to avoid unavailables on startup (CASSANDRA-16783) * Fix clustering order logic in CREATE MATERIALIZED VIEW (CASSANDRA-16898) diff --git a/src/java/org/apache/cassandra/tools/nodetool/stats/StatsPrinter.java b/src/java/org/apache/cassandra/tools/nodetool/stats/StatsPrinter.java index e67f33afec..389efba9ac 100644 --- a/src/java/org/apache/cassandra/tools/nodetool/stats/StatsPrinter.java +++ b/src/java/org/apache/cassandra/tools/nodetool/stats/StatsPrinter.java @@ -18,9 +18,10 @@ package org.apache.cassandra.tools.nodetool.stats; +import java.io.IOException; import java.io.PrintStream; -import org.json.simple.JSONObject; +import com.fasterxml.jackson.databind.ObjectMapper; import org.yaml.snakeyaml.DumperOptions; import org.yaml.snakeyaml.Yaml; @@ -39,9 +40,17 @@ public interface StatsPrinter @Override public void print(T data, PrintStream out) { - JSONObject json = new JSONObject(); - json.putAll(data.convert2Map()); - out.println(json.toString()); + ObjectMapper mapper = new ObjectMapper(); + try + { + String json = mapper.writerWithDefaultPrettyPrinter() + .writeValueAsString(data.convert2Map()); + out.println(json); + } + catch (IOException e) + { + out.println(e); + } } } diff --git a/src/java/org/apache/cassandra/tools/nodetool/stats/TpStatsHolder.java b/src/java/org/apache/cassandra/tools/nodetool/stats/TpStatsHolder.java index 38011baaad..db9c298865 100644 --- a/src/java/org/apache/cassandra/tools/nodetool/stats/TpStatsHolder.java +++ b/src/java/org/apache/cassandra/tools/nodetool/stats/TpStatsHolder.java @@ -18,7 +18,6 @@ package org.apache.cassandra.tools.nodetool.stats; -import java.util.Arrays; import java.util.HashMap; import java.util.Map; @@ -39,7 +38,7 @@ public class TpStatsHolder implements StatsHolder HashMap result = new HashMap<>(); HashMap> threadPools = new HashMap<>(); HashMap droppedMessage = new HashMap<>(); - HashMap waitLatencies = new HashMap<>(); + HashMap waitLatencies = new HashMap<>(); for (Map.Entry tp : probe.getThreadPools().entries()) { @@ -55,13 +54,11 @@ public class TpStatsHolder implements StatsHolder for (Map.Entry entry : probe.getDroppedMessages().entrySet()) { - droppedMessage.put(entry.getKey(), entry.getValue()); + String key = entry.getKey(); + droppedMessage.put(key, entry.getValue()); try { - String[] strValues = (String[]) Arrays.stream(probe.metricPercentilesAsArray(probe.getMessagingQueueWaitMetrics(entry.getKey()))) - .map(D -> D.toString()) - .toArray(); - waitLatencies.put(entry.getKey(), strValues); + waitLatencies.put(key, probe.metricPercentilesAsArray(probe.getMessagingQueueWaitMetrics(key))); } catch (RuntimeException e) { diff --git a/test/unit/org/apache/cassandra/tools/NodeToolTPStatsTest.java b/test/unit/org/apache/cassandra/tools/NodeToolTPStatsTest.java index 31423a407d..b507bcd31e 100644 --- a/test/unit/org/apache/cassandra/tools/NodeToolTPStatsTest.java +++ b/test/unit/org/apache/cassandra/tools/NodeToolTPStatsTest.java @@ -44,6 +44,7 @@ import org.assertj.core.api.Assertions; import org.yaml.snakeyaml.Yaml; import static org.apache.cassandra.net.Verb.ECHO_REQ; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; @@ -68,6 +69,7 @@ public class NodeToolTPStatsTest extends CQLTester } @Test + @SuppressWarnings("SingleCharacterStringConcatenation") public void testMaybeChangeDocs() { // If you added, modified options or help, please update docs if necessary @@ -151,18 +153,22 @@ public class NodeToolTPStatsTest extends CQLTester } @Test - public void testFromatArg() throws Throwable + public void testFormatArg() { Arrays.asList(Pair.of("-F", "json"), Pair.of("--format", "json")).forEach(arg -> { ToolResult tool = ToolRunner.invokeNodetool("tpstats", arg.getLeft(), arg.getRight()); - assertTrue(isJSONString(tool.getStdout())); + String json = tool.getStdout(); + assertThat(isJSONString(json)).isTrue(); + assertThat(json).containsPattern("\"WaitLatencies\"\\s*:\\s*\\{\\s*\""); assertTrue(tool.getCleanedStderr().isEmpty()); assertEquals(0, tool.getExitCode()); }); Arrays.asList( Pair.of("-F", "yaml"), Pair.of("--format", "yaml")).forEach(arg -> { ToolResult tool = ToolRunner.invokeNodetool("tpstats", arg.getLeft(), arg.getRight()); - assertTrue(isYAMLString(tool.getStdout())); + String yaml = tool.getStdout(); + assertThat(isYAMLString(yaml)).isTrue(); + assertThat(yaml).containsPattern("WaitLatencies:\\s*[A-Z|_]+:\\s+-\\s"); assertTrue(tool.getCleanedStderr().isEmpty()); assertEquals(0, tool.getExitCode()); }); diff --git a/test/unit/org/apache/cassandra/tools/nodetool/stats/StatsPrinterTest.java b/test/unit/org/apache/cassandra/tools/nodetool/stats/StatsPrinterTest.java new file mode 100644 index 0000000000..0cf2ea798d --- /dev/null +++ b/test/unit/org/apache/cassandra/tools/nodetool/stats/StatsPrinterTest.java @@ -0,0 +1,186 @@ +/* + * 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.cassandra.tools.nodetool.stats; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.PrintStream; +import java.util.Arrays; +import java.util.Map; +import java.util.TreeMap; +import java.util.regex.Pattern; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +/** + * Tests for {@link StatsPrinter}. + */ +public class StatsPrinterTest +{ + @Test + public void testPrintEmpty() throws Throwable + { + TestCase.create() + .validateJson("{}") + .validateYaml("{}"); + } + + @Test + public void testPrintSimpleTypes() throws Throwable + { + TestCase.create("null", null) + .validateJson("{\"null\":null}") + .validateYaml("'null': null"); + TestCase.create("string", "") + .validateJson("{\"string\":\"\"}") + .validateYaml("string: ''"); + TestCase.create("string", "test") + .validateJson("{\"string\":\"test\"}") + .validateYaml("string: test"); + TestCase.create("bool", true) + .validateJson("{\"bool\":true}") + .validateYaml("bool: true"); + TestCase.create("bool", false) + .validateJson("{\"bool\":false}") + .validateYaml("bool: false"); + TestCase.create("int", 1) + .validateJson("{\"int\":1}") + .validateYaml("int: 1"); + TestCase.create("long", 1L) + .validateJson("{\"long\":1}") + .validateYaml("long: 1"); + TestCase.create("float", 1.1f) + .validateJson("{\"float\":1.1}") + .validateYaml("float: 1.1"); + TestCase.create("double", 1.1d) + .validateJson("{\"double\":1.1}") + .validateYaml("double: 1.1"); + } + + @Test + public void testPrintArrays() throws Throwable + { + TestCase.create("ints", new int[]{ -1, 0, 1 }) + .validateJson("{\"ints\":[-1, 0, 1]}") + .validateYaml("ints:\n- -1\n- 0\n- 1"); + TestCase.create("longs", new long[]{ -1L, 0L, 1L }) + .validateJson("{\"longs\":[-1, 0, 1]}") + .validateYaml("longs:\n- -1\n- 0\n- 1"); + TestCase.create("floats", new float[]{ -1.1f, 0.1f, 1.1f }) + .validateJson("{\"floats\":[-1.1, 0.1, 1.1]}") + .validateYaml("floats:\n- -1.1\n- 0.1\n- 1.1"); + TestCase.create("doubles", new double[]{ -1.1d, 0.1d, 1.1d }) + .validateJson("{\"doubles\":[-1.1, 0.1, 1.1]}") + .validateYaml("doubles:\n- -1.1\n- 0.1\n- 1.1"); + } + + @Test + public void testPrintLists() throws Throwable + { + TestCase.create("ints", Arrays.asList(-1, 0, 1)) + .validateJson("{\"ints\":[-1,0,1]}") + .validateYaml("ints:\n- -1\n- 0\n- 1"); + TestCase.create("longs", Arrays.asList(-1L, 0L, 1L)) + .validateJson("{\"longs\":[-1,0,1]}") + .validateYaml("longs:\n- -1\n- 0\n- 1"); + TestCase.create("floats", Arrays.asList(-1.1f, 0.1f, 1.1f)) + .validateJson("{\"floats\":[-1.1,0.1,1.1]}") + .validateYaml("floats:\n- -1.1\n- 0.1\n- 1.1"); + TestCase.create("doubles", Arrays.asList(-1.1d, 0.1d, 1.1d)) + .validateJson("{\"doubles\":[-1.1,0.1,1.1]}") + .validateYaml("doubles:\n- -1.1\n- 0.1\n- 1.1"); + } + + @Test + public void testPrintMultiple() throws Throwable + { + TestCase.create() + .put("string", "test") + .put("array", new int[]{ -1, 0, 1 }) + .put("list", Arrays.asList(-1, 0, 1)) + .validateJson("{\"array\":[-1,0,1],\"list\":[-1,0,1],\"string\":\"test\"}") + .validateYaml("array:\n- -1\n- 0\n- 1\nlist:\n- -1\n- 0\n- 1\nstring: test"); + } + + private static class TestCase implements StatsHolder + { + private static final StatsPrinter jsonPrinter = new StatsPrinter.JsonPrinter<>(); + private static final StatsPrinter yamlPrinter = new StatsPrinter.YamlPrinter<>(); + + private final Map map; + + private TestCase() + { + this.map = new TreeMap<>(); + } + + public static TestCase create() + { + return new TestCase(); + } + + public static TestCase create(String key, Object value) + { + return create().put(key, value); + } + + public TestCase put(String key, Object value) + { + map.put(key, value); + return this; + } + + @Override + public Map convert2Map() + { + return map; + } + + private String print(StatsPrinter printer) throws IOException + { + try (ByteArrayOutputStream stream = new ByteArrayOutputStream()) + { + printer.print(this, new PrintStream(stream)); + return stream.toString(); + } + } + + private static String cleanJson(String json) + { + return Pattern.compile("\n|\\s*").matcher(json).replaceAll(""); + } + + TestCase validateJson(String expectedJson) throws IOException + { + String expected = cleanJson(expectedJson); + String actual = cleanJson(print(jsonPrinter)); + assertEquals(expected, actual); + return this; + } + + @SuppressWarnings("UnusedReturnValue") + TestCase validateYaml(String expectedYaml) throws IOException + { + assertEquals(expectedYaml + "\n\n", print(yamlPrinter)); + return this; + } + } +}