Skip to content

CASSANDRA-21605 Improve algorithmic complexity for finding intersecting SSTables for LCS compaction candidates - #5061

Open
alanwang67 wants to merge 10 commits into
apache:trunkfrom
alanwang67:compactionCandidate
Open

CASSANDRA-21605 Improve algorithmic complexity for finding intersecting SSTables for LCS compaction candidates#5061
alanwang67 wants to merge 10 commits into
apache:trunkfrom
alanwang67:compactionCandidate

Conversation

@alanwang67

Copy link
Copy Markdown
Contributor

For LCS, SSTables in levels 1 through level 8 are already stored in a tree set and are disjoint. However when we look for compaction candidates we do a linear scan of the entire set to find intersecting SSTables. We can instead just take the floor of the tree set and iterate forwards until we no longer find any intersections.

patch by Alan Wang;

reviewed by for CASSANDRA-21605

Co-authored-by: Name1
Co-authored-by: Name2

@krummas krummas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me, very small comments inline

Comment thread src/java/org/apache/cassandra/db/compaction/LeveledManifest.java Outdated
SSTableReader start = sstablesNextLevel.floor(sstable);
Iterator<SSTableReader> it = sstablesNextLevel.tailSet(start != null ? start : sstablesNextLevel.first(), true).iterator();

while (it.hasNext()) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit; put the brace on newline

return Collections.emptyList();
}

public static Set<SSTableReader> getIntersectingSSTablesFromTreeSet(SSTableReader sstable, TreeSet<SSTableReader> sstablesNextLevel)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be package protected and @VisibleForTesting

@@ -592,6 +596,26 @@ private Collection<SSTableReader> getCandidatesFor(int level)
return Collections.emptyList();
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add some javadoc to describe that sstable will be included in the result

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored this so that we add the sstable outside of getIntersectingSSTablesFromTreeSet to make it less confusing.

ColumnFamilyStore cfs = MockSchema.newCFS();
List<SSTableReader> sstables = new ArrayList<>();

long seed = System.currentTimeMillis();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

print/log the seed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did this within the assert when it fails.

Comment thread test/unit/org/apache/cassandra/db/compaction/LeveledCompactionStrategyTest.java Outdated
Comment thread test/unit/org/apache/cassandra/db/compaction/LeveledCompactionStrategyTest.java Outdated
@alanwang67
alanwang67 requested a review from krummas August 27, 2026 17:58
Comment thread src/java/org/apache/cassandra/db/compaction/LeveledManifest.java Outdated
Comment thread test/unit/org/apache/cassandra/db/compaction/LeveledCompactionStrategyTest.java Outdated
Comment thread test/unit/org/apache/cassandra/db/compaction/LeveledCompactionStrategyTest.java Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants