[
https://issues.apache.org/jira/browse/CASSANDRA-16772?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18114516#comment-18114516
]
Mykyta Bozhenko commented on CASSANDRA-16772:
---------------------------------------------
Patch: https://github.com/apache/cassandra/pull/5153
The grouping map is HashMap<ColumnFamilyStore, Descriptor>, so when several
files of the same table are passed, each put overwrites the previous one and
only the last file is cleaned; the rest are dropped without a warning. The
patch uses Descriptor.fromFilenamesGrouped(), the Multimap helper you pointed
at, which forceUserDefinedCompaction already uses, and iterates every
descriptor per table. Two small things in the same method: the isJoined() check
moves ahead of filename parsing, and a null transaction from tryModify (the
SSTable is already compacting) is logged and skipped instead of being
dereferenced.
Test: a two-token ring where the local node owns three of six partitions and
three SSTables hold two partitions each; cleanup is requested for two of them.
Both requested originals must be replaced by outputs containing exactly the
owned partition, and the third SSTable must stay untouched. Against unpatched
production it fails with "Requested original must be replaced: .../pa-1-big",
i.e. the first requested file is left alone.
You noted in 2021 that forceUserDefinedCleanup is not wired to nodetool and may
be dead code. I checked trunk: there is still no production caller, it is
reachable only over JMX through CompactionManagerMBean. So the more useful
question may be whether this should be fixed or removed; the patch is there
either way.
> User Defined nodetool cleanup only processes one SSTable per table
> ------------------------------------------------------------------
>
> Key: CASSANDRA-16772
> URL: https://issues.apache.org/jira/browse/CASSANDRA-16772
> Project: Apache Cassandra
> Issue Type: Bug
> Components: Local/SSTable
> Reporter: Scott Carey
> Assignee: Scott Carey
> Priority: Normal
> Fix For: 3.11.x
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> User defined nodetool cleanup uses a HashMap instead of a MultiMap to group
> the user provided SSTables by table. This means it only keeps one file per
> source table.
> It also means the unit test for this component is not sufficient.
> As part of https://issues.apache.org/jira/browse/CASSANDRA-16767 I
> introduced a helper method on Descriptor:
> {code:java}
> public static Multimap<ColumnFamilyStore, Descriptor>
> fromFilenamesGrouped(Collection<String> filenames) {code}
> That should be used instead of the custom logic in
> CompactionManager.forceUserDefinedCleanup.
>
> Broken existing code:
> {code:java}
> HashMap<ColumnFamilyStore, Descriptor> descriptors =
> Maps.newHashMap(); for (String filename : filenames)
> {
> // extract keyspace and columnfamily name from filename
> Descriptor desc = Descriptor.fromFilename(filename.trim());
> if (Schema.instance.getCFMetaData(desc) == null)
> {
> logger.warn("Schema does not exist for file {}. Skipping.",
> filename);
> continue;
> }
> // group by keyspace/columnfamily
> ColumnFamilyStore cfs =
> Keyspace.open(desc.ksname).getColumnFamilyStore(desc.cfname);
> desc = cfs.getDirectories().find(new
> File(filename.trim()).getName());
> if (desc != null)
> descriptors.put(cfs, desc);
> } {code}
>
> Contents of helper method introduced in other ticket:
> {code:java}
> public static Multimap<ColumnFamilyStore, Descriptor>
> fromFilenamesGrouped(Collection<String> filenames) {
> Multimap<ColumnFamilyStore, Descriptor> descriptors =
> ArrayListMultimap.create(); for (String filename : filenames)
> {
> // extract keyspace and columnfamily name from filename
> Descriptor desc = Descriptor.fromFilename(filename.trim());
> if (Schema.instance.getCFMetaData(desc) == null)
> {
> logger.warn("Schema does not exist for file {}. Skipping.",
> filename);
> continue;
> }
> // group by keyspace/columnfamily
> ColumnFamilyStore cfs =
> Keyspace.open(desc.ksname).getColumnFamilyStore(desc.cfname);
> desc = cfs.getDirectories().find(new
> File(filename.trim()).getName());
> if (desc != null)
> descriptors.put(cfs, desc);
> }
> return descriptors;
> } {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
