Merge branch 'cassandra-4.0' into trunk

CHANGES.txt

@@ -10,6 +10,7 @@
  * Add a system property to set hostId if not yet initialized (CASSANDRA-14582)
  * GossiperTest.testHasVersion3Nodes didn't take into account trunk version changes, fixed to rely on latest version (CASSANDRA-16651)
 Merged from 4.0:
+ * Improve help, doc and error messages about sstabledump -k and -x arguments (CASSANDRA-16818)
  * Add repaired/unrepaired bytes back to nodetool (CASSANDRA-15282)
  * Upgrade lz4-java to 1.8.0 to add RH6 support back (CASSANDRA-16753)
  * Improve DiagnosticEventService.publish(event) logging message of events (CASSANDRA-16749)

doc/source/tools/sstable/sstabledump.rst

@@ -30,7 +30,7 @@ sstabledump <options> <sstable file path>
 ===================================     ================================================================================
 -d                                      CQL row per line internal representation
 -e                                      Enumerate partition keys only
--k <arg>                                Partition key
+-k <arg>                                Included partition key(s)
 -x <arg>                                Excluded partition key(s)
 -t                                      Print raw timestamps instead of iso8601 date strings
 -l                                      Output each row as a separate JSON object
@@ -174,7 +174,7 @@ Example::
 Dump only keys
 ^^^^^^^^^^^^^^
 
-Dump only the keys by using the -e option.
+Dump only the partition keys by using the -e option.
 
 Example::
 
@@ -183,14 +183,16 @@ Example::
     cat eventlog_dump_2018Jul26b
     [ [ "3578d7de-c60d-4599-aefb-3f22a07b2bc6" ], [ "d18250c0-84fc-4d40-b957-4248dc9d790e" ], [ "cf188983-d85b-48d6-9365-25005289beb2" ]
 
-Dump row for a single key
+Dump rows with some partition key or keys
-^^^^^^^^^^^^^^^^^^^^^^^^^
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-Dump a single key using the -k option.
+Dump a set of rows identified by their partition keys using the -k option. Multiple keys can be used.
+
+Please note that the -k option should be after the sstable path.
 
 Example::
 
-    sstabledump /var/lib/cassandra/data/keyspace/eventlog-65c429e08c5a11e8939edf4f403979ef/mc-1-big-Data.db -k 3578d7de-c60d-4599-aefb-3f22a07b2bc6 > eventlog_dump_2018Jul26_singlekey
+    sstabledump /var/lib/cassandra/data/keyspace/eventlog-65c429e08c5a11e8939edf4f403979ef/mc-1-big-Data.db -k 3578d7de-c60d-4599-aefb-3f22a07b2bc6 d18250c0-84fc-4d40-b957-4248dc9d790e > eventlog_dump_2018Jul26_singlekey
 
     cat eventlog_dump_2018Jul26_singlekey
     [
@@ -211,12 +213,32 @@ Example::
             ]
           }
         ]
+      },
+      {
+        "partition" : {
+          "key" : [ "d18250c0-84fc-4d40-b957-4248dc9d790e" ],
+          "position" : 62
+        },
+        "rows" : [
+          {
+            "type" : "row",
+            "position" : 123,
+            "liveness_info" : { "tstamp" : "2018-07-20T20:23:07.783522Z" },
+            "cells" : [
+              { "name" : "event", "value" : "party" },
+              { "name" : "insertedtimestamp", "value" : "2018-07-20 20:23:07.789Z" },
+              { "name" : "source", "value" : "asdf" }
+            ]
+          }
+        ]
       }
 
-Exclude a key or keys in dump of rows
+Exclude a partition key or keys in dump of rows
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-Dump a table except for the rows excluded with the -x option. Multiple keys can be used.
+Dump a table except for the rows excluded with the -x option. Multiple partition keys can be used.
+
+Please note that the -x option should be after the sstable path.
 
 Example::
 

src/java/org/apache/cassandra/tools/SSTableExport.java

@@ -73,12 +73,12 @@ public class SSTableExport
     {
         DatabaseDescriptor.clientInitialization();
 
-        Option optKey = new Option(KEY_OPTION, true, "Partition key");
+        Option optKey = new Option(KEY_OPTION, true, "List of included partition keys");
         // Number of times -k <key> can be passed on the command line.
         optKey.setArgs(500);
         options.addOption(optKey);
 
-        Option excludeKey = new Option(EXCLUDE_KEY_OPTION, true, "Excluded partition key");
+        Option excludeKey = new Option(EXCLUDE_KEY_OPTION, true, "List of excluded partition keys");
         // Number of times -x <key> can be passed on the command line.
         excludeKey.setArgs(500);
         options.addOption(excludeKey);
@@ -119,18 +119,22 @@ public class SSTableExport
             System.exit(1);
         }
 
-        if (cmd.getArgs().length != 1)
-        {
-            System.err.println("You must supply exactly one sstable");
-            printUsage();
-            System.exit(1);
-        }
-
         String[] keys = cmd.getOptionValues(KEY_OPTION);
         HashSet<String> excludes = new HashSet<>(Arrays.asList(
                 cmd.getOptionValues(EXCLUDE_KEY_OPTION) == null
                         ? new String[0]
                         : cmd.getOptionValues(EXCLUDE_KEY_OPTION)));
+
+        if (cmd.getArgs().length != 1)
+        {
+            String msg = "You must supply exactly one sstable";
+            if (cmd.getArgs().length == 0 && (keys != null && keys.length > 0 || !excludes.isEmpty()))
+                msg += ", which should be before the -k/-x options so it's not interpreted as a partition key.";
+
+            System.err.println(msg);
+            printUsage();
+            System.exit(1);
+        }
         String ssTableFileName = new File(cmd.getArgs()[0]).getAbsolutePath();
 
         if (!new File(ssTableFileName).exists())

test/unit/org/apache/cassandra/tools/SSTableExportTest.java

@@ -22,6 +22,7 @@ import java.io.IOException;
 import java.util.List;
 import java.util.Map;
 
+import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
@@ -31,9 +32,13 @@ import com.fasterxml.jackson.databind.exc.MismatchedInputException;
 import org.apache.cassandra.OrderedJUnit4ClassRunner;
 import org.apache.cassandra.tools.ToolRunner.ToolResult;
 import org.assertj.core.api.Assertions;
-import org.hamcrest.CoreMatchers;
 
+import static org.hamcrest.CoreMatchers.containsStringIgnoringCase;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.CoreMatchers.startsWith;
+import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
@@ -41,22 +46,25 @@ import static org.junit.Assert.fail;
 @RunWith(OrderedJUnit4ClassRunner.class)
 public class SSTableExportTest extends OfflineToolUtils
 {
-    private ObjectMapper mapper = new ObjectMapper();
+    private static final ObjectMapper mapper = new ObjectMapper();
-    private TypeReference<List<Map<String, Object>>> jacksonListOfMapsType = new TypeReference<List<Map<String, Object>>>() {};
+    private static final TypeReference<List<Map<String, Object>>> jacksonListOfMapsType = new TypeReference<List<Map<String, Object>>>() {};
+    private static String sstable;
+
+    @BeforeClass
+    public static void setupTest() throws IOException
+    {
+        sstable = findOneSSTable("legacy_sstables", "legacy_ma_simple");
+    }
 
     @Test
     public void testNoArgsPrintsHelp()
     {
         ToolResult tool = ToolRunner.invokeClass(SSTableExport.class);
-        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("usage:"));
+        assertThat(tool.getStdout(), containsStringIgnoringCase("usage:"));
-        assertThat(tool.getCleanedStderr(), CoreMatchers.containsStringIgnoringCase("You must supply exactly one sstable"));
+        assertThat(tool.getCleanedStderr(), containsStringIgnoringCase("You must supply exactly one sstable"));
+        assertThat(tool.getCleanedStderr(), not(containsStringIgnoringCase("before the -k/-x options")));
         assertEquals(1, tool.getExitCode());
-        assertNoUnexpectedThreadsStarted(null, OPTIONAL_THREADS_WITH_SCHEMA);
+        assertPostTestEnv(false);
-        assertSchemaNotLoaded();
-        assertCLSMNotLoaded();
-        assertSystemKSNotLoaded();
-        assertKeyspaceNotLoaded();
-        assertServerNotLoaded();
     }
 
     @Test
@@ -64,106 +72,156 @@ public class SSTableExportTest extends OfflineToolUtils
     {
         // If you added, modified options or help, please update docs if necessary
         ToolResult tool = ToolRunner.invokeClass(SSTableExport.class);
-        String help = "usage: sstabledump <sstable file path> <options>\n" + 
+        String help = "usage: sstabledump <sstable file path> <options>\n" +
-                       "                   \n" + 
+                       "                   \n" +
-                       "Dump contents of given SSTable to standard output in JSON format.\n" + 
+                       "Dump contents of given SSTable to standard output in JSON format.\n" +
-                       " -d         CQL row per line internal representation\n" + 
+                       " -d         CQL row per line internal representation\n" +
-                       " -e         enumerate partition keys only\n" + 
+                       " -e         enumerate partition keys only\n" +
-                       " -k <arg>   Partition key\n" + 
+                       " -k <arg>   List of included partition keys\n" +
-                       " -l         Output json lines, by partition\n" + 
+                       " -l         Output json lines, by partition\n" +
-                       " -t         Print raw timestamps instead of iso8601 date strings\n" + 
+                       " -t         Print raw timestamps instead of iso8601 date strings\n" +
-                       " -x <arg>   Excluded partition key\n";
+                       " -x <arg>   List of excluded partition keys\n";
         Assertions.assertThat(tool.getStdout()).isEqualTo(help);
+        assertPostTestEnv(false);
     }
 
     @Test
-    public void testWrongArgFailsAndPrintsHelp() throws IOException
+    public void testWrongArgFailsAndPrintsHelp()
     {
-        ToolResult tool = ToolRunner.invokeClass(SSTableExport.class, "--debugwrong", findOneSSTable("legacy_sstables", "legacy_ma_simple"));
+        ToolResult tool = ToolRunner.invokeClass(SSTableExport.class, "--debugwrong", sstable);
-        assertThat(tool.getStdout(), CoreMatchers.containsStringIgnoringCase("usage:"));
+        assertThat(tool.getStdout(), containsStringIgnoringCase("usage:"));
-        assertThat(tool.getCleanedStderr(), CoreMatchers.containsStringIgnoringCase("Unrecognized option"));
+        assertThat(tool.getCleanedStderr(), containsStringIgnoringCase("Unrecognized option"));
         assertEquals(1, tool.getExitCode());
+        assertPostTestEnv(false);
     }
 
     @Test
     public void testDefaultCall() throws IOException
     {
-        ToolResult tool = ToolRunner.invokeClass(SSTableExport.class,findOneSSTable("legacy_sstables", "legacy_ma_simple"));
+        ToolResult tool = ToolRunner.invokeClass(SSTableExport.class, sstable);
         List<Map<String, Object>> parsed = mapper.readValue(tool.getStdout(), jacksonListOfMapsType);
-        assertTrue(tool.getStdout(), parsed.get(0).get("partition") != null);
+        assertNotNull(tool.getStdout(), parsed.get(0).get("partition"));
-        assertTrue(tool.getStdout(), parsed.get(0).get("rows") != null);
+        assertNotNull(tool.getStdout(), parsed.get(0).get("rows"));
         Assertions.assertThat(tool.getCleanedStderr()).isEmpty();
         tool.assertOnExitCode();
-        assertPostTestEnv();
+        assertPostTestEnv(true);
     }
 
     @Test
-    public void testCQLRowArg() throws IOException
+    public void testCQLRowArg()
     {
-        ToolResult tool = ToolRunner.invokeClass(SSTableExport.class, findOneSSTable("legacy_sstables", "legacy_ma_simple"), "-d");
+        ToolResult tool = ToolRunner.invokeClass(SSTableExport.class, sstable, "-d");
-        assertThat(tool.getStdout(), CoreMatchers.startsWith("[0]"));
+        assertThat(tool.getStdout(), startsWith("[0]"));
         Assertions.assertThat(tool.getCleanedStderr()).isEmpty();
         tool.assertOnExitCode();
-        assertPostTestEnv();
+        assertPostTestEnv(true);
     }
 
     @Test
-    public void testPKOnlyArg() throws IOException
+    public void testPKOnlyArg()
     {
-        ToolResult tool = ToolRunner.invokeClass(SSTableExport.class, findOneSSTable("legacy_sstables", "legacy_ma_simple"), "-e");
+        ToolResult tool = ToolRunner.invokeClass(SSTableExport.class, sstable, "-e");
         assertEquals(tool.getStdout(), "[ [ \"0\" ], [ \"1\" ], [ \"2\" ], [ \"3\" ], [ \"4\" ]\n]", tool.getStdout());
         Assertions.assertThat(tool.getCleanedStderr()).isEmpty();
         tool.assertOnExitCode();
-        assertPostTestEnv();
+        assertPostTestEnv(true);
     }
 
     @Test
     public void testPKArg() throws IOException
     {
-        ToolResult tool = ToolRunner.invokeClass(SSTableExport.class, findOneSSTable("legacy_sstables", "legacy_ma_simple"), "-k", "0");
+        ToolResult tool = ToolRunner.invokeClass(SSTableExport.class, sstable, "-k", "0");
-        List<Map<String, Object>> parsed = mapper.readValue(tool.getStdout(), jacksonListOfMapsType);
+        assertKeys(tool, "0");
-        assertEquals(tool.getStdout(), 1, parsed.size());
+        assertPostTestEnv(true);
-        assertEquals(tool.getStdout(), "0", ((List) ((Map) parsed.get(0).get("partition")).get("key")).get(0));
+    }
-        Assertions.assertThat(tool.getCleanedStderr()).isEmpty();
+
-        tool.assertOnExitCode();
+    @Test
-        assertPostTestEnv();
+    public void testPKArgOutOfOrder()
+    {
+        ToolResult tool = ToolRunner.invokeClass(SSTableExport.class, "-k", "0", sstable);
+        assertThat(tool.getStdout(), containsStringIgnoringCase("usage:"));
+        assertThat(tool.getCleanedStderr(), containsStringIgnoringCase("You must supply exactly one sstable"));
+        assertThat(tool.getCleanedStderr(), containsStringIgnoringCase("before the -k/-x options"));
+        assertEquals(1, tool.getExitCode());
+        assertPostTestEnv(false);
+    }
+
+    @Test
+    public void testMultiplePKArg() throws IOException
+    {
+        ToolResult tool = ToolRunner.invokeClass(SSTableExport.class, sstable, "-k", "0", "2");
+        assertKeys(tool, "0", "2");
+        assertPostTestEnv(true);
     }
 
     @Test
     public void testExcludePKArg() throws IOException
     {
-        ToolResult tool = ToolRunner.invokeClass(SSTableExport.class, findOneSSTable("legacy_sstables", "legacy_ma_simple"), "-x", "0");
+        ToolResult tool = ToolRunner.invokeClass(SSTableExport.class, sstable, "-x", "0");
-        List<Map<String, Object>> parsed = mapper.readValue(tool.getStdout(), jacksonListOfMapsType);
+        assertKeys(tool, "1", "2", "3", "4");
-        assertEquals(tool.getStdout(), 4, parsed.size());
+        assertPostTestEnv(true);
-        Assertions.assertThat(tool.getCleanedStderr()).isEmpty();
-        tool.assertOnExitCode();
-        assertPostTestEnv();
     }
 
     @Test
+    public void testExcludePKArgOutOfOrder()
+    {
+        ToolResult tool = ToolRunner.invokeClass(SSTableExport.class, "-x", "0", sstable);
+        assertThat(tool.getStdout(), containsStringIgnoringCase("usage:"));
+        assertThat(tool.getCleanedStderr(), containsStringIgnoringCase("You must supply exactly one sstable"));
+        assertThat(tool.getCleanedStderr(), containsStringIgnoringCase("before the -k/-x options"));
+        assertEquals(1, tool.getExitCode());
+        assertPostTestEnv(false);
+    }
+
+    @Test
+    public void testMultipleExcludePKArg() throws IOException
+    {
+        ToolResult tool = ToolRunner.invokeClass(SSTableExport.class, sstable, "-x", "0", "2");
+        assertKeys(tool, "1", "3", "4");
+        assertPostTestEnv(true);
+    }
+
+    @SuppressWarnings("rawtypes")
+    private void assertKeys(ToolResult tool, String... expectedKeys) throws IOException
+    {
+        List<Map<String, Object>> parsed = mapper.readValue(tool.getStdout(), jacksonListOfMapsType);
+        String[] actualKeys = parsed.stream()
+                                    .map(x -> (Map) x.get("partition"))
+                                    .map(x -> (List) x.get("key"))
+                                    .map(x -> (String) x.get(0))
+                                    .toArray(String[]::new);
+        assertArrayEquals(expectedKeys, actualKeys);
+        Assertions.assertThat(tool.getCleanedStderr()).isEmpty();
+        tool.assertOnExitCode();
+    }
+
+    @Test
+    @SuppressWarnings({ "rawtypes", "unchecked" })
     public void testTSFormatArg() throws IOException
     {
-        ToolResult tool = ToolRunner.invokeClass(SSTableExport.class, findOneSSTable("legacy_sstables", "legacy_ma_simple"), "-t");
+        ToolResult tool = ToolRunner.invokeClass(SSTableExport.class, sstable, "-t");
         List<Map<String, Object>> parsed = mapper.readValue(tool.getStdout(), jacksonListOfMapsType);
         assertEquals(tool.getStdout(),
                      "1445008632854000",
                      ((Map) ((List<Map>) parsed.get(0).get("rows")).get(0).get("liveness_info")).get("tstamp"));
         Assertions.assertThat(tool.getCleanedStderr()).isEmpty();
         tool.assertOnExitCode();
-        assertPostTestEnv();
+        assertPostTestEnv(true);
     }
 
     @Test
+    @SuppressWarnings({ "rawtypes", "DynamicRegexReplaceableByCompiledPattern" })
     public void testJSONLineArg() throws IOException
     {
-        ToolResult tool = ToolRunner.invokeClass(SSTableExport.class, findOneSSTable("legacy_sstables", "legacy_ma_simple"), "-l");
+        ToolResult tool = ToolRunner.invokeClass(SSTableExport.class, sstable, "-l");
         try
         {
             mapper.readValue(tool.getStdout(), jacksonListOfMapsType);
             fail("Shouldn't be able to deserialize that output, now it's not a collection anymore.");
         }
-        catch(MismatchedInputException e)
+        catch (MismatchedInputException e)
         {
+            assertThat(e.getMessage(), startsWith("Cannot deserialize"));
         }
 
         int parsedCount = 0;
@@ -175,17 +233,22 @@ public class SSTableExportTest extends OfflineToolUtils
         }
 
         assertEquals(tool.getStdout(), 5, parsedCount);
-        assertThat(tool.getStdout(), CoreMatchers.startsWith("{\""));
+        assertThat(tool.getStdout(), startsWith("{\""));
         Assertions.assertThat(tool.getCleanedStderr()).isEmpty();
         tool.assertOnExitCode();
-        assertPostTestEnv();
+        assertPostTestEnv(true);
     }
 
-    private void assertPostTestEnv()
+    private void assertPostTestEnv(boolean loadsSchema)
     {
         assertNoUnexpectedThreadsStarted(null, OPTIONAL_THREADS_WITH_SCHEMA);
+        // schema loading seems to depend of the JVM version,
+        // so we only verify the cases where we are sure it's not loaded
+        if (!loadsSchema)
+            assertSchemaNotLoaded();
         assertCLSMNotLoaded();
         assertSystemKSNotLoaded();
+        assertKeyspaceNotLoaded();
         assertServerNotLoaded();
     }
 }