SQUASH – delete SSTableHeaderFix altogether, BigFormat.earliest_suppo…

…rted_version applies to both online and offline

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

@@ -46,7 +46,6 @@
 import org.apache.cassandra.db.lifecycle.LifecycleTransaction;
 import org.apache.cassandra.io.sstable.Component;
 import org.apache.cassandra.io.sstable.Descriptor;
-import org.apache.cassandra.io.sstable.SSTableHeaderFix;
 import org.apache.cassandra.io.sstable.format.SSTableReader;
 import org.apache.cassandra.io.util.File;
 import org.apache.cassandra.schema.Schema;
@@ -129,66 +128,6 @@ public static void main(String args[])
             }
             System.out.println(String.format("Pre-scrub sstables snapshotted into snapshot %s", snapshotName));
 
-            if (options.headerFixMode != Options.HeaderFixMode.OFF)
-            {
-                // Run the frozen-UDT checks _before_ the sstables are opened
-
-                List<String> logOutput = new ArrayList<>();
-
-                SSTableHeaderFix.Builder headerFixBuilder = SSTableHeaderFix.builder()
-                                                                            .logToList(logOutput)
-                                                                            .schemaCallback(() -> Schema.instance::getTableMetadata);
-                if (options.headerFixMode == Options.HeaderFixMode.VALIDATE)
-                    headerFixBuilder = headerFixBuilder.dryRun();
-
-                for (Pair<Descriptor, Set<Component>> p : listResult)
-                    headerFixBuilder.withPath(p.left.fileFor(Component.DATA).toPath());
-
-                SSTableHeaderFix headerFix = headerFixBuilder.build();
-                try
-                {
-                    headerFix.execute();
-                }
-                catch (Exception e)
-                {
-                    JVMStabilityInspector.inspectThrowable(e);
-                    if (options.debug)
-                        e.printStackTrace(System.err);
-                }
-
-                if (headerFix.hasChanges() || headerFix.hasError())
-                    logOutput.forEach(System.out::println);
-
-                if (headerFix.hasError())
-                {
-                    System.err.println("Errors in serialization-header detected, aborting.");
-                    System.exit(1);
-                }
-
-                switch (options.headerFixMode)
-                {
-                    case VALIDATE_ONLY:
-                    case FIX_ONLY:
-                        System.out.printf("Not continuing with scrub, since '--%s %s' was specified.%n",
-                                          HEADERFIX_OPTION,
-                                          options.headerFixMode.asCommandLineOption());
-                        System.exit(0);
-                    case VALIDATE:
-                        if (headerFix.hasChanges())
-                        {
-                            System.err.printf("Unfixed, but fixable errors in serialization-header detected, aborting. " +
-                                              "Use a non-validating mode ('-e %s' or '-e %s') for --%s%n",
-                                              Options.HeaderFixMode.FIX.asCommandLineOption(),
-                                              Options.HeaderFixMode.FIX_ONLY.asCommandLineOption(),
-                                              HEADERFIX_OPTION);
-                            System.exit(2);
-                        }
-                        break;
-                    case FIX:
-                        break;
-                }
-            }
-
             List<SSTableReader> sstables = new ArrayList<>();
 
             // Open sstables
@@ -286,9 +225,9 @@ private static class Options
         public boolean skipCorrupted;
         public boolean noValidate;
         public boolean reinserOverflowedTTL;
-        public HeaderFixMode headerFixMode = HeaderFixMode.VALIDATE;
 
-        enum HeaderFixMode
+        @Deprecated // sstables that need the fix are no longer supported.  kept for cmdline compatibility (can remove in 6.0)
+        private enum HeaderFixMode
         {
             VALIDATE_ONLY,
             VALIDATE,
@@ -300,11 +239,6 @@ static HeaderFixMode fromCommandLine(String value)
             {
                 return valueOf(value.replace('-', '_').toUpperCase().trim());
             }
-
-            String asCommandLineOption()
-            {
-                return name().toLowerCase().replace('_', '-');
-            }
         }
 
         private Options(String keyspaceName, String cfName)
@@ -347,11 +281,11 @@ public static Options parseArgs(String cmdArgs[])
                 opts.skipCorrupted = cmd.hasOption(SKIP_CORRUPTED_OPTION);
                 opts.noValidate = cmd.hasOption(NO_VALIDATE_OPTION);
                 opts.reinserOverflowedTTL = cmd.hasOption(REINSERT_OVERFLOWED_TTL_OPTION);
-                if (cmd.hasOption(HEADERFIX_OPTION))
-                {
+                if (cmd.hasOption(HEADERFIX_OPTION)) {
                     try
                     {
-                        opts.headerFixMode = HeaderFixMode.fromCommandLine(cmd.getOptionValue(HEADERFIX_OPTION));
+                        HeaderFixMode.fromCommandLine(cmd.getOptionValue(HEADERFIX_OPTION));
+                        System.out.println(String.format("The option %s is deprecated (and does nothing)", HEADERFIX_OPTION));
                     }
                     catch (Exception e)
                     {
@@ -384,22 +318,7 @@ private static CmdLineOptions getCmdLineOptions()
             options.addOption("m",  MANIFEST_CHECK_OPTION, "only check and repair the leveled manifest, without actually scrubbing the sstables");
             options.addOption("s",  SKIP_CORRUPTED_OPTION, "skip corrupt rows in counter tables");
             options.addOption("n",  NO_VALIDATE_OPTION,    "do not validate columns using column validator");
-            options.addOption("e",  HEADERFIX_OPTION,      true, "Option whether and how to perform a " +
-                                                                 "check of the sstable serialization-headers and fix known, " +
-                                                                 "fixable issues.\n" +
-                                                                 "Possible argument values:\n" +
-                                                                 "- validate-only: validate the serialization-headers, " +
-                                                                 "but do not fix those. Do not continue with scrub - " +
-                                                                 "i.e. only validate the header (dry-run of fix-only).\n" +
-                                                                 "- validate: (default) validate the serialization-headers, " +
-                                                                 "but do not fix those and only continue with scrub if no " +
-                                                                 "error were detected.\n" +
-                                                                 "- fix-only: validate and fix the serialization-headers, " +
-                                                                 "don't continue with scrub.\n" +
-                                                                 "- fix: validate and fix the serialization-headers, do not " +
-                                                                 "fix and do not continue with scrub if the serialization-header " +
-                                                                 "check encountered errors.\n" +
-                                                                 "- off: don't perform the serialization-header checks.");
+            options.addOption("e",  HEADERFIX_OPTION,      true, "Deprecated Option.");
             options.addOption("r", REINSERT_OVERFLOWED_TTL_OPTION, REINSERT_OVERFLOWED_TTL_OPTION_DESCRIPTION);
             return options;
         }

test/unit/org/apache/cassandra/cql3/validation/operations/TTLTest.java

@@ -50,7 +50,6 @@
 
 public class TTLTest extends CQLTester
 {
-    public static String NEGATIVE_LOCAL_EXPIRATION_TEST_DIR = "test/data/negative-local-expiration-test/%s";
 
     public static int MAX_TTL = Attributes.MAX_TTL;
 
@@ -204,20 +203,6 @@ public void testRejectExpirationOverflowPolicy() throws Throwable
         }
     }
 
-    @Test
-    public void testRecoverOverflowedExpirationWithScrub() throws Throwable
-    {
-        // this tests writes corrupt tombstones on purpose, disable the strategy:
-        DatabaseDescriptor.setCorruptedTombstoneStrategy(Config.CorruptedTombstoneStrategy.disabled);
-        baseTestRecoverOverflowedExpiration(false, false, false);
-        baseTestRecoverOverflowedExpiration(true, false, false);
-        baseTestRecoverOverflowedExpiration(true, false, true);
-
-        baseTestRecoverOverflowedExpiration(false, true, false);
-        baseTestRecoverOverflowedExpiration(false, true, true);
-        // we reset the corrupted ts strategy after each test in @After above
-    }
-
     public void testCapExpirationDateOverflowPolicy(ExpirationDateOverflowHandling.ExpirationDateOverflowPolicy policy) throws Throwable
     {
         ExpirationDateOverflowHandling.policy = policy;
@@ -285,18 +270,6 @@ public void testCapExpirationDateOverflowPolicy(boolean simple, boolean clusteri
         }
     }
 
-    public void baseTestRecoverOverflowedExpiration(boolean runScrub, boolean runSStableScrub, boolean reinsertOverflowedTTL) throws Throwable
-    {
-        // simple column, clustering
-        testRecoverOverflowedExpirationWithScrub(true, true, runScrub, runSStableScrub, reinsertOverflowedTTL);
-        // simple column, noclustering
-        testRecoverOverflowedExpirationWithScrub(true, false, runScrub, runSStableScrub, reinsertOverflowedTTL);
-        // complex column, clustering
-        testRecoverOverflowedExpirationWithScrub(false, true, runScrub, runSStableScrub, reinsertOverflowedTTL);
-        // complex column, noclustering
-        testRecoverOverflowedExpirationWithScrub(false, false, runScrub, runSStableScrub, reinsertOverflowedTTL);
-    }
-
     private void createTable(boolean simple, boolean clustering)
     {
         if (simple)
@@ -359,100 +332,6 @@ private int computeMaxTTL()
         return AbstractCell.MAX_DELETION_TIME - nowInSecs;
     }
 
-    public void testRecoverOverflowedExpirationWithScrub(boolean simple, boolean clustering, boolean runScrub, boolean runSStableScrub,  boolean reinsertOverflowedTTL) throws Throwable
-    {
-        if (reinsertOverflowedTTL)
-        {
-            assert runScrub || runSStableScrub;
-        }
-
-        createTable(simple, clustering);
-
-        Keyspace keyspace = Keyspace.open(KEYSPACE);
-        ColumnFamilyStore cfs = keyspace.getColumnFamilyStore(currentTable());
-
-        assertEquals(0, cfs.getLiveSSTables().size());
-
-        copySSTablesToTableDir(currentTable(), simple, clustering);
-
-        cfs.loadNewSSTables();
-
-        if (runScrub)
-        {
-            cfs.scrub(true, false, true, reinsertOverflowedTTL, 1);
-
-            if (reinsertOverflowedTTL)
-            {
-                if (simple)
-                    assertRows(execute("SELECT * from %s"), row(1, 1, 1), row(2, 2, null));
-                else
-                    assertRows(execute("SELECT * from %s"), row(1, 1, set("v11", "v12", "v13", "v14")), row(2, 2, set("v21", "v22", "v23", "v24")));
-
-                cfs.forceMajorCompaction();
-
-                if (simple)
-                    assertRows(execute("SELECT * from %s"), row(1, 1, 1), row(2, 2, null));
-                else
-                    assertRows(execute("SELECT * from %s"), row(1, 1, set("v11", "v12", "v13", "v14")), row(2, 2, set("v21", "v22", "v23", "v24")));
-            }
-            else
-            {
-                assertEmpty(execute("SELECT * from %s"));
-            }
-        }
-        if (runSStableScrub)
-        {
-            System.setProperty(org.apache.cassandra.tools.Util.ALLOW_TOOL_REINIT_FOR_TEST, "true"); // Necessary for testing
-
-            try
-            {
-                ToolResult tool;
-                if (reinsertOverflowedTTL)
-                    tool = ToolRunner.invokeClass(StandaloneScrubber.class, "-r", KEYSPACE, cfs.name);
-                else
-                    tool = ToolRunner.invokeClass(StandaloneScrubber.class, KEYSPACE, cfs.name);
-
-                tool.assertOnCleanExit();
-                Assertions.assertThat(tool.getStdout()).contains("Pre-scrub sstables snapshotted into");
-                if (reinsertOverflowedTTL)
-                    Assertions.assertThat(tool.getStdout()).contains("Fixed 2 rows with overflowed local deletion time.");
-                else
-                    Assertions.assertThat(tool.getStdout()).contains("No valid partitions found while scrubbing");
-            }
-            finally
-            {
-                System.clearProperty(org.apache.cassandra.tools.Util.ALLOW_TOOL_REINIT_FOR_TEST);
-            }
-        }
-
-        try
-        {
-            cfs.truncateBlocking();
-            dropTable("DROP TABLE %s");
-        }
-        catch (Throwable e)
-        {
-            // StandaloneScrubber.class should be ran as a tool with a stable env. In a test env there are things moving
-            // under its feet such as the async CQLTester.afterTest() operations. We try to sync cleanup of tables here
-            // but we need to catch any exceptions we might run into bc of the hack. See CASSANDRA-16546
-        }
-    }
-
-    private void copySSTablesToTableDir(String table, boolean simple, boolean clustering) throws IOException
-    {
-        File destDir = Keyspace.open(keyspace()).getColumnFamilyStore(table).getDirectories().getCFDirectories().iterator().next();
-        File sourceDir = getTableDir(table, simple, clustering);
-        for (File file : sourceDir.tryList())
-        {
-            copyFile(file, destDir);
-        }
-    }
-
-    private static File getTableDir(String table, boolean simple, boolean clustering)
-    {
-        return new File(String.format(NEGATIVE_LOCAL_EXPIRATION_TEST_DIR, getTableName(simple, clustering)));
-    }
-
     private static void copyFile(File src, File dest) throws IOException
     {
         byte[] buf = new byte[65536];