diff --git a/CHANGES.txt b/CHANGES.txt index 80ee45ca2e..927c4234cb 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.1 + * Fix missed wait latencies in the output of `nodetool tpstats -F` (CASSANDRA-16938) * Reduce native transport max frame size to 16MB (CASSANDRA-16886) * Add support for filtering using IN restrictions (CASSANDRA-14344) * Provide a nodetool command to invalidate auth caches (CASSANDRA-16404) 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/nodetool/TpStatsTest.java b/test/unit/org/apache/cassandra/tools/nodetool/TpStatsTest.java index 0cbcb496f1..edbd6b3edb 100644 --- a/test/unit/org/apache/cassandra/tools/nodetool/TpStatsTest.java +++ b/test/unit/org/apache/cassandra/tools/nodetool/TpStatsTest.java @@ -141,18 +141,22 @@ public class TpStatsTest extends CQLTester } @Test - public void testFromatArg() + public void testFormatArg() { Arrays.asList(Pair.of("-F", "json"), Pair.of("--format", "json")).forEach(arg -> { ToolRunner.ToolResult tool = ToolRunner.invokeNodetool("tpstats", arg.getLeft(), arg.getRight()); tool.assertOnCleanExit(); - assertThat(isJSONString(tool.getStdout())).isTrue(); + String json = tool.getStdout(); + assertThat(isJSONString(json)).isTrue(); + assertThat(json).containsPattern("\"WaitLatencies\"\\s*:\\s*\\{\\s*\""); }); Arrays.asList( Pair.of("-F", "yaml"), Pair.of("--format", "yaml")).forEach(arg -> { ToolRunner.ToolResult tool = ToolRunner.invokeNodetool("tpstats", arg.getLeft(), arg.getRight()); tool.assertOnCleanExit(); - assertThat(isYAMLString(tool.getStdout())).isTrue(); + String yaml = tool.getStdout(); + assertThat(isYAMLString(yaml)).isTrue(); + assertThat(yaml).containsPattern("WaitLatencies:\\s*[A-Z|_]+:\\s+-\\s"); }); } 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; + } + } +}