diff --git a/src/java/com/palantir/cassandra/db/ColumnFamilyStoreManager.java b/src/java/com/palantir/cassandra/db/ColumnFamilyStoreManager.java index 291aa37f01..177139edd2 100644 --- a/src/java/com/palantir/cassandra/db/ColumnFamilyStoreManager.java +++ b/src/java/com/palantir/cassandra/db/ColumnFamilyStoreManager.java @@ -62,16 +62,6 @@ public Map> filterValidAncestors(CFMetaData cfMetaData, return validator.filterValidAncestors(cfMetaData, sstableToCompletedAncestors, unfinishedCompactions); } - @Override - public boolean shouldRemoveUnusedSstablesBasedOnAncestorMetadata() { - return validator.shouldRemoveUnusedSstablesBasedOnAncestorMetadata(); - } - - @Override - public boolean shouldSkipAncestorCleanupBasedOnAncestorMetadata() { - return validator.shouldSkipAncestorCleanupBasedOnAncestorMetadata(); - } - @Override public void markForDeletion(CFMetaData cfMetaData, Set descriptors) { diff --git a/src/java/com/palantir/cassandra/db/IColumnFamilyStoreValidator.java b/src/java/com/palantir/cassandra/db/IColumnFamilyStoreValidator.java index c569692ab3..6493b75969 100644 --- a/src/java/com/palantir/cassandra/db/IColumnFamilyStoreValidator.java +++ b/src/java/com/palantir/cassandra/db/IColumnFamilyStoreValidator.java @@ -33,22 +33,4 @@ public interface IColumnFamilyStoreValidator */ Map> filterValidAncestors(CFMetaData cfMetaData, Map> sstableToCompletedAncestors, Map unfinishedCompactions); - - /** - * @return true if Cassandra should use ancestry metdata to cleanup unused SSTables on startup by running - * {@link org.apache.cassandra.db.ColumnFamilyStore#removeUnusedSstables(CFMetaData, Map)}, false otherwise - * (e.g. if a different cleanup system is being used outside of {@link org.apache.cassandra.service.CassandraDaemon}). - */ - default boolean shouldRemoveUnusedSstablesBasedOnAncestorMetadata() { - return true; - } - - /** - * @return true if Cassandra should skip cleaning up ancestors during - * {@link org.apache.cassandra.db.ColumnFamilyStore#removeUnusedSstables(CFMetaData, Map)}, false otherwise. Note - * that this flag does not control whether compaction products being cleaned up. - */ - default boolean shouldSkipAncestorCleanupBasedOnAncestorMetadata() { - return false; - } } diff --git a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java index fcdc18a3eb..331dd821c2 100644 --- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java +++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java @@ -665,138 +665,6 @@ public boolean accept(File pathname) } } - /** - * Replacing compacted sstables is atomic as far as observers of Tracker are concerned, but not on the - * filesystem: first the new sstables are renamed to "live" status (i.e., the tmp marker is removed), then - * their ancestors are removed. - * - * If an unclean shutdown happens at the right time, we can thus end up with both the new ones and their - * ancestors "live" in the system. This is harmless for normal data, but for counters it can cause overcounts. - * - * To prevent this, we record sstables being compacted in the system keyspace. If we find unfinished - * compactions, we remove the new ones (since those may be incomplete -- under LCS, we may create multiple - * sstables from any given ancestor). - */ - public static void removeUnusedSstables(CFMetaData metadata, Map unfinishedCompactions) - { - Directories directories = new Directories(metadata); - Set allGenerations = new HashSet<>(); - for (Descriptor desc : directories.sstableLister().list().keySet()) - allGenerations.add(desc.generation); - - // sanity-check unfinishedCompactions - Set unfinishedGenerations = unfinishedCompactions.keySet(); - if (!allGenerations.containsAll(unfinishedGenerations)) - { - HashSet missingGenerations = new HashSet<>(unfinishedGenerations); - missingGenerations.removeAll(allGenerations); - logger.info("Unfinished compactions reference missing sstables of generations", - SafeArg.of("keyspace", metadata.ksName), SafeArg.of("cf", metadata.cfName), - SafeArg.of("missingGenerations", missingGenerations)); - } - - // remove new sstables from compactions that didn't complete, and compute - // set of ancestors that shouldn't exist anymore - Map> allSstableToAncestors = new HashMap<>(); - Set completedAncestors = new HashSet<>(); - Map> allNonTempSstableFiles = directories.sstableLister().skipTemporary(true).list(); - for (Map.Entry> sstableFiles : allNonTempSstableFiles.entrySet()) - { - // we rename the Data component last - if it does not exist as a final file, we should ignore this sstable and - // it will be removed during startup - if (!sstableFiles.getValue().contains(Component.DATA)) - continue; - - Descriptor desc = sstableFiles.getKey(); - - Set ancestors; - try - { - CompactionMetadata compactionMetadata = (CompactionMetadata) desc.getMetadataSerializer().deserialize(desc, MetadataType.COMPACTION); - ancestors = compactionMetadata.ancestors; - } - catch (IOException e) - { - throw new FSReadError(e, desc.filenameFor(Component.STATS)); - } - catch (NullPointerException e) - { - throw new FSReadError(e, "Failed to remove unfinished compaction leftovers (file: " + desc.filenameFor(Component.STATS) + "). See log for details."); - } - allSstableToAncestors.put(desc, ancestors); - } - - allSstableToAncestors = ColumnFamilyStoreManager.instance.filterValidAncestors(metadata, allSstableToAncestors, unfinishedCompactions); - SafeArg>> ancestorsArg = SafeArg.of( - "sstableToAncestors", - allSstableToAncestors.entrySet().stream() - .collect(Collectors.toMap( - (Map.Entry> e) -> e.getKey().generation, - Map.Entry::getValue))); - - Set cleanedUnfinishedCompactions = new HashSet<>(); - for (Map.Entry> sstableToAncestors : allSstableToAncestors.entrySet()) - { - Descriptor desc = sstableToAncestors.getKey(); - Set ancestors = sstableToAncestors.getValue(); - if (!ancestors.isEmpty() - && unfinishedGenerations.containsAll(ancestors) - && allGenerations.containsAll(ancestors)) - { - // any of the ancestors would work, so we'll just lookup the compaction task ID with the first one - UUID compactionTaskID = unfinishedCompactions.get(ancestors.iterator().next()); - assert compactionTaskID != null; - if (DISABLE_COMPACTION_PRODUCT_CLEANUP) - { - logger.info("Would have deleted unfinished compaction product", UnsafeArg.of("desc", desc), - SafeArg.of("keyspace", desc.ksname), SafeArg.of("cf", desc.cfname), - SafeArg.of("generation", desc.generation), ancestorsArg); - } - else - { - logger.info("Going to delete unfinished compaction product", UnsafeArg.of("desc", desc), - SafeArg.of("keyspace", desc.ksname), SafeArg.of("cf", desc.cfname), - SafeArg.of("generation", desc.generation), ancestorsArg); - SSTable.delete(desc, allNonTempSstableFiles.get(desc)); - } - cleanedUnfinishedCompactions.add(compactionTaskID); - } - else - { - completedAncestors.addAll(ancestors); - } - } - cleanedUnfinishedCompactions.forEach(SystemKeyspace::finishCompaction); - - if (ColumnFamilyStoreManager.instance.shouldSkipAncestorCleanupBasedOnAncestorMetadata()) { - return; - } - - // remove old sstables from compactions that did complete - for (Map.Entry> sstableFiles : directories.sstableLister().list().entrySet()) - { - Descriptor desc = sstableFiles.getKey(); - if (completedAncestors.contains(desc.generation)) - { - if (DRY_RUN_NON_COMPACTING_UNUSED_SSTABLE_CLEANUP && unfinishedCompactions.isEmpty()) - { - logger.warn("Would have deleted leftover compaction ancestor", UnsafeArg.of("desc", desc), - SafeArg.of("keyspace", desc.ksname), SafeArg.of("cf", desc.cfname), - SafeArg.of("generation", desc.generation), ancestorsArg); - } else - { - // if any of the ancestors were participating in a compaction, finish that compaction - logger.warn("Going to delete leftover compaction ancestor", UnsafeArg.of("desc", desc), - SafeArg.of("keyspace", desc.ksname), SafeArg.of("cf", desc.cfname), - SafeArg.of("generation", desc.generation), ancestorsArg); - SSTable.delete(desc, sstableFiles.getValue()); - Optional.ofNullable(unfinishedCompactions.get(desc.generation)) - .ifPresent(SystemKeyspace::finishCompaction); - } - } - } - } - /** * See #{@code StorageService.loadNewSSTables(String, String)} for more info * diff --git a/src/java/org/apache/cassandra/service/CassandraDaemon.java b/src/java/org/apache/cassandra/service/CassandraDaemon.java index 6aaa491cec..3ddba27168 100644 --- a/src/java/org/apache/cassandra/service/CassandraDaemon.java +++ b/src/java/org/apache/cassandra/service/CassandraDaemon.java @@ -284,11 +284,6 @@ private void completeSetupMayThrowSstableException() { Map, Map> unfinishedCompactions = SystemKeyspace.getUnfinishedCompactions(); for (String keyspaceName : Schema.instance.getKeyspaces()) { - for (CFMetaData cfm : Schema.instance.getKeyspaceMetaData(keyspaceName).values()) - { - ColumnFamilyStore.removeUnusedSstables(cfm, unfinishedCompactions.getOrDefault(cfm.ksAndCFName, ImmutableMap.of())); - } - if (keyspaceName.equals(SystemKeyspace.NAME)) continue; diff --git a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java b/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java index 1930be774d..8c9bbe3023 100644 --- a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java +++ b/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java @@ -119,9 +119,6 @@ public class ColumnFamilyStoreTest public static final String CF_STANDARD4 = "Standard4"; public static final String CF_STANDARD5 = "Standard5"; public static final String CF_STANDARD6 = "Standard6"; - public static final String CF_STANDARD7 = "Standard7"; - public static final String CF_STANDARD8 = "Standard8"; - public static final String CF_STANDARD9 = "Standard9"; public static final String CF_STANDARDINT = "StandardInteger1"; public static final String CF_SUPER1 = "Super1"; public static final String CF_SUPER6 = "Super6"; @@ -151,9 +148,6 @@ public static void defineSchema() throws ConfigurationException SchemaLoader.standardCFMD(KEYSPACE1, CF_STANDARD4), SchemaLoader.standardCFMD(KEYSPACE1, CF_STANDARD5), SchemaLoader.standardCFMD(KEYSPACE1, CF_STANDARD6), - SchemaLoader.standardCFMD(KEYSPACE1, CF_STANDARD7), - SchemaLoader.standardCFMD(KEYSPACE1, CF_STANDARD8), - SchemaLoader.standardCFMD(KEYSPACE1, CF_STANDARD9), SchemaLoader.indexCFMD(KEYSPACE1, CF_INDEX1, true), SchemaLoader.indexCFMD(KEYSPACE1, CF_INDEX2, false), SchemaLoader.superCFMD(KEYSPACE1, CF_SUPER1, LongType.instance), @@ -1841,256 +1835,6 @@ public void testMultipleRangesSlicesInMemory() throws Throwable testMultiRangeSlicesBehavior(prepareMultiRangeSlicesTest(10, false)); } - @Test - public void testRemoveUnusedSstables() throws Throwable - { - String ks = KEYSPACE1; - String cf = CF_STANDARD3; // should be empty - - final CFMetaData cfmeta = Schema.instance.getCFMetaData(ks, cf); - Directories dir = new Directories(cfmeta); - ByteBuffer key = bytes("key"); - - // 1st sstable - SSTableSimpleWriter writer = new SSTableSimpleWriter(dir.getDirectoryForNewSSTables(), cfmeta, StorageService.getPartitioner()); - writer.newRow(key); - writer.addColumn(bytes("col"), bytes("val"), 1); - writer.close(); - - Map> sstables = dir.sstableLister().list(); - assertEquals(1, sstables.size()); - - Map.Entry> sstableToOpen = sstables.entrySet().iterator().next(); - final SSTableReader sstable1 = SSTableReader.open(sstableToOpen.getKey()); - - // simulate incomplete compaction - writer = new SSTableSimpleWriter(dir.getDirectoryForNewSSTables(), - cfmeta, StorageService.getPartitioner()) - { - protected SSTableWriter getWriter() - { - MetadataCollector collector = new MetadataCollector(cfmeta.comparator); - collector.addAncestor(sstable1.descriptor.generation); // add ancestor from previously written sstable - return SSTableWriter.create(createDescriptor(directory, metadata.ksName, metadata.cfName, DatabaseDescriptor.getSSTableFormat()), - 0L, - ActiveRepairService.UNREPAIRED_SSTABLE, - metadata, - DatabaseDescriptor.getPartitioner(), - collector); - } - }; - writer.newRow(key); - writer.addColumn(bytes("col"), bytes("val"), 1); - writer.close(); - - // should have 2 sstables now - sstables = dir.sstableLister().list(); - assertEquals(2, sstables.size()); - - SSTableReader sstable2 = SSTableReader.open(sstable1.descriptor); - UUID compactionTaskID = SystemKeyspace.startCompaction( - Keyspace.open(ks).getColumnFamilyStore(cf), - Collections.singleton(sstable2)); - - Map unfinishedCompaction = new HashMap<>(); - unfinishedCompaction.put(sstable1.descriptor.generation, compactionTaskID); - ColumnFamilyStore.removeUnusedSstables(cfmeta, unfinishedCompaction); - - // 2nd sstable should be removed (only 1st sstable exists in set of size 1) - sstables = dir.sstableLister().list(); - assertEquals(1, sstables.size()); - assertTrue(sstables.containsKey(sstable1.descriptor)); - - Map, Map> unfinished = SystemKeyspace.getUnfinishedCompactions(); - assertTrue(unfinished.isEmpty()); - sstable1.selfRef().release(); - sstable2.selfRef().release(); - } - - /** - * @see CASSANDRA-6086 - */ - @Test - public void testFailedToRemoveUnusedSstables() throws Throwable - { - final String ks = KEYSPACE1; - final String cf = CF_STANDARD4; // should be empty - - final CFMetaData cfmeta = Schema.instance.getCFMetaData(ks, cf); - Directories dir = new Directories(cfmeta); - ByteBuffer key = bytes("key"); - - // Write SSTable generation 3 that has ancestors 1 and 2 - final Set ancestors = Sets.newHashSet(1, 2); - SSTableSimpleWriter writer = new SSTableSimpleWriter(dir.getDirectoryForNewSSTables(), - cfmeta, StorageService.getPartitioner()) - { - protected SSTableWriter getWriter() - { - MetadataCollector collector = new MetadataCollector(cfmeta.comparator); - for (int ancestor : ancestors) - collector.addAncestor(ancestor); - String file = new Descriptor(directory, ks, cf, 3, Descriptor.Type.TEMP).filenameFor(Component.DATA); - return SSTableWriter.create(Descriptor.fromFilename(file), - 0L, - ActiveRepairService.UNREPAIRED_SSTABLE, - metadata, - StorageService.getPartitioner(), - collector); - } - }; - writer.newRow(key); - writer.addColumn(bytes("col"), bytes("val"), 1); - writer.close(); - - Map> sstables = dir.sstableLister().list(); - assert sstables.size() == 1; - - Map.Entry> sstableToOpen = sstables.entrySet().iterator().next(); - final SSTableReader sstable1 = SSTableReader.open(sstableToOpen.getKey()); - - // simulate we don't have generation in compaction_history - Map unfinishedCompactions = new HashMap<>(); - UUID compactionTaskID = UUID.randomUUID(); - for (Integer ancestor : ancestors) - unfinishedCompactions.put(ancestor, compactionTaskID); - ColumnFamilyStore.removeUnusedSstables(cfmeta, unfinishedCompactions); - - // SSTable should not be deleted - sstables = dir.sstableLister().list(); - assert sstables.size() == 1; - assert sstables.containsKey(sstable1.descriptor); - } - - @Test - public void testRemoveUnusedSstablesOnlyRemovesFiltered() throws IOException - { - final String ks = KEYSPACE1; - final String cf = CF_STANDARD7; - - final CFMetaData cfmeta = Schema.instance.getCFMetaData(ks, cf); - Keyspace.open(KEYSPACE1).getColumnFamilyStore(cf).disableAutoCompaction(); - Directories dir = new Directories(cfmeta); - - int gen1 = writeNextGenerationSstable(ImmutableSet.of(), dir, cfmeta); - int gen2 = writeNextGenerationSstable(ImmutableSet.of(), dir, cfmeta); - int gen3 = writeNextGenerationSstable(ImmutableSet.of(gen1, gen2), dir, cfmeta); - int gen4 = writeNextGenerationSstable(ImmutableSet.of(), dir, cfmeta); - int gen5 = writeNextGenerationSstable(ImmutableSet.of(gen4), dir, cfmeta); - - Map> sstables = dir.sstableLister().list(); - Descriptor sstable3Desc = sstables.keySet().iterator().next().withGeneration(gen3); - assertEquals(5, sstables.size()); - assertTrue(sstables.containsKey(sstable3Desc)); - - IColumnFamilyStoreValidator validator = (_cfMetaData, sstableToCompletedAncestors, _unfinishedCompactions) -> { - Set allowedGenerations = ImmutableSet.of(gen1, gen2, gen4, gen5); - return sstableToCompletedAncestors.entrySet().stream() - .filter(entry -> allowedGenerations.contains(entry.getKey().generation)) - .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); - }; - - try { - ColumnFamilyStoreManager.instance.registerValidator(validator); - ColumnFamilyStore.removeUnusedSstables(cfmeta, ImmutableMap.of()); - } - finally - { - ColumnFamilyStoreManager.instance.unregisterValidator(); - } - - sstables = dir.sstableLister().list(); - ImmutableSet expected = ImmutableSet.of( - sstable3Desc.withGeneration(gen1), - sstable3Desc.withGeneration(gen2), - sstable3Desc.withGeneration(gen3), - sstable3Desc.withGeneration(gen5)); - assertEquals(expected, sstables.keySet()); - } - - @Test - public void testRemoveUnusedSstablesDoesNotTouchProductsWhenAncestorDoesNotExist() throws IOException - { - final String ks = KEYSPACE1; - final String cf = CF_STANDARD8; - - final CFMetaData cfmeta = Schema.instance.getCFMetaData(ks, cf); - Keyspace.open(KEYSPACE1).getColumnFamilyStore(cf).disableAutoCompaction(); - Directories dir = new Directories(cfmeta); - - int gen1 = writeNextGenerationSstable(ImmutableSet.of(), dir, cfmeta); - int gen2 = writeNextGenerationSstable(ImmutableSet.of(), dir, cfmeta); - int gen3 = writeNextGenerationSstable(ImmutableSet.of(gen1, gen2), dir, cfmeta); - int gen4 = writeNextGenerationSstable(ImmutableSet.of(), dir, cfmeta); - int gen5 = writeNextGenerationSstable(ImmutableSet.of(gen4), dir, cfmeta); - - Map> sstables = dir.sstableLister().list(); - Descriptor sstable3Desc = sstables.keySet().iterator().next().withGeneration(gen3); - assertEquals(5, sstables.size()); - assertTrue(sstables.containsKey(sstable3Desc)); - - Files.delete(Paths.get(sstables.keySet().iterator().next().withGeneration(gen3).filenameFor(Component.DATA))); - - ColumnFamilyStore.removeUnusedSstables(cfmeta, ImmutableMap.of()); - - sstables = dir.sstableLister().list(); - ImmutableSet products = ImmutableSet.of( - sstable3Desc.withGeneration(gen3), - sstable3Desc.withGeneration(gen5)); - assertTrue(sstables.keySet().containsAll(products)); - } - - @Test - public void testRemoveUnusedSstablesDoesNotAncestorsWhenManagerSkips() throws IOException - { - final String ks = KEYSPACE1; - final String cf = CF_STANDARD9; - - final CFMetaData cfmeta = Schema.instance.getCFMetaData(ks, cf); - Keyspace.open(KEYSPACE1).getColumnFamilyStore(cf).disableAutoCompaction(); - Directories dir = new Directories(cfmeta); - - int gen1 = writeNextGenerationSstable(ImmutableSet.of(), dir, cfmeta); - int gen2 = writeNextGenerationSstable(ImmutableSet.of(), dir, cfmeta); - int gen3 = writeNextGenerationSstable(ImmutableSet.of(gen1, gen2), dir, cfmeta); - int gen4 = writeNextGenerationSstable(ImmutableSet.of(), dir, cfmeta); - int gen5 = writeNextGenerationSstable(ImmutableSet.of(gen4), dir, cfmeta); - - Map> sstables = dir.sstableLister().list(); - Descriptor sstable3Desc = sstables.keySet().iterator().next().withGeneration(gen3); - assertEquals(5, sstables.size()); - assertTrue(sstables.containsKey(sstable3Desc)); - - IColumnFamilyStoreValidator validator = new IColumnFamilyStoreValidator() - { - public Map> filterValidAncestors(CFMetaData cfMetaData, Map> sstableToCompletedAncestors, Map unfinishedCompactions) - { - return sstableToCompletedAncestors; - } - - public boolean shouldSkipAncestorCleanupBasedOnAncestorMetadata() - { - return true; - } - }; - - try { - ColumnFamilyStoreManager.instance.registerValidator(validator); - ColumnFamilyStore.removeUnusedSstables(cfmeta, ImmutableMap.of()); - } - finally - { - ColumnFamilyStoreManager.instance.unregisterValidator(); - } - - sstables = dir.sstableLister().list(); - ImmutableSet ancestors = ImmutableSet.of( - sstable3Desc.withGeneration(gen1), - sstable3Desc.withGeneration(gen2), - sstable3Desc.withGeneration(gen4)); - assertTrue(sstables.keySet().containsAll(ancestors)); - } - @Test public void testLoadNewSSTablesAvoidsOverwrites() throws Throwable {