CNDB-14577: Compact all SSTables of a level shard if their number reaches a limit #1873

cbornet · 2025-07-15T12:58:47Z

What is the issue

CNDB-14577: UCS by default does not compact many small non-overlapping sstables with very few rows

What does this PR fix and why was it fixed

This PR limits the number of SSTable for a given compaction level by executing a major compaction of the level instead of the regular compaction of overlapping SSTables.

TODO:

add docs

github-actions · 2025-07-15T12:59:03Z

blambov

Added a couple of comments, and I need to take a fresh look at how the parallelism is controlled on Monday to give you some ideas for that.

src/java/org/apache/cassandra/db/compaction/UnifiedCompactionStrategy.java

blambov · 2025-07-21T07:04:45Z

AFAICS there are no issues with the parallelism, it should be properly handled by getSelection.

blambov · 2025-07-23T13:32:42Z

src/java/org/apache/cassandra/db/compaction/UnifiedCompactionStrategy.java

+                // If that's the case, we perform a major compaction on those shards.
+                List<Set<CompactionSSTable>> groups =
+                shardManager.splitSSTablesInShards(sstables,
+                                                   new ShardingStats(sstables, shardManager, controller).shardCountForDensity,


Nit: This ShardingStats collection is rather costly. Since we already have a density range for the level, let's use a simpler controller.getNumShards(max).

blambov · 2025-07-23T13:37:28Z

src/java/org/apache/cassandra/db/compaction/UnifiedCompactionStrategy.java

+                                                   (sstableShard, shardRange) -> Sets.newHashSet(sstableShard));
+
+                List<Set<CompactionSSTable>> oversizeGroups =
+                groups.stream()


Nit: I'm not a fan of the lack of indentation here. Could we move these a few spaces to the right?

It comes from the style guide that was set by ant generate-idea-files:

cassandra/ide/idea/codeStyleSettings.xml

Line 67 in 4182c62

<option name="CONTINUATION_INDENT_SIZE" value="0" />

This is there and makes a lot of sense for the cases of

x -> { ... }

or

functionCall ( ... )

but

x = something

is breaking readability. I'm afraid we may have to adjust these manually.

These can't be configured separately in IDEA ?
I'll have a look.

blambov · 2025-07-23T13:45:29Z

src/java/org/apache/cassandra/db/compaction/UnifiedCompactionStrategy.java

+
+                List<Set<CompactionSSTable>> oversizeGroups =
+                groups.stream()
+                      .filter(group -> group.size() > threshold * controller.getShardMaxSstablesFactor())


I think it makes better sense to use the fan factor instead of the threshold (the threshold is always 2 for levelled compaction, and it feels wrong/surprising for e.g. T4 and L4 to have different triggers).

blambov · 2025-07-23T13:49:45Z

src/java/org/apache/cassandra/db/compaction/UnifiedCompactionStrategy.java

+                for (Bucket bucket : buckets)
+                    aggregates.add(bucket.constructAggregate(controller, spaceAvailable, arena));
+            }
+            else


Let's add an if (sstables.size() > limit) to skip quickly if we don't have enough sstables for any shard to be over the limit.

blambov · 2025-07-23T14:00:38Z

src/java/org/apache/cassandra/db/compaction/unified/Controller.java

@@ -314,6 +314,10 @@ public abstract class Controller
    @Deprecated
    static final String STATIC_SCALING_FACTORS_OPTION = "static_scaling_factors";

+    static final String SHARD_MAX_SSTABLES_FACTOR_OPTION = "shard_max_sstables_factor";


The name isn't very readable to me.

Maybe max_sstables_per_shard_factor or sstables_per_shard_max_factor?

Or non_overlapping_threshold/trigger?

Note that we also need to document this in UnifiedCompactionStrategy.md -- both as a section that describes the behaviour as well as a paragraph in the configuration section. If we can't find a default that works well in almost all situations, also make sure it is eventually added to the product documentation.

blambov · 2025-07-23T14:04:15Z

src/java/org/apache/cassandra/db/compaction/UnifiedCompactionStrategy.java

+                    for (Set<CompactionSSTable> group : groups)
+                    {
+                        if (group.stream().anyMatch(
+                        sstable -> oversizeGroups.stream().anyMatch(oversizeGroup -> oversizeGroup.contains(sstable))))


AFAIK we can't use streams in OSS C*, where this will also need to be ported. Perhaps turn these into loops already here (IntelliJ can do this for you) so that the code does not immediately diverge?

Is this a performance concern ?
Because the min JDK seems to be 11 even for OSS C*

Performance. Apparently it is not yet officially forbidden, but it will be: https://lists.apache.org/thread/65glsjzkmpktzmns6j9wvr4nczvskx36

I'm also afraid this can fall under the current "don't use streams on hot paths" recommendation.

…ches a limit

blambov

The code looks good to me, and we can proceed to make an image from it.

If you think you won't have the time to do the documentation additions, let me know and I will prepare a doc commit.

blambov · 2025-07-24T10:44:17Z

src/java/org/apache/cassandra/db/compaction/UnifiedCompactionStrategy.java

-                                                              Collections.emptyList(),
-                                                              arena,
-                                                              this));
+                            oversizeGroups.add(compactionSSTables);


Could you add a comment explaining why you prefer to save the groups and check them individually instead of adding all to a single set?
(So that someone reading it doesn't immediately want to do an optimization that may be counterproductive.)

Well, this optimization would be good, wouldn't it ?
I think I should do it...

eolivelli · 2025-07-24T12:12:48Z

how do we disable this feature ?
like setting max_sstables_per_shard_factor to a very big number ?

cbornet · 2025-07-24T12:18:08Z

how do we disable this feature ?
like setting max_sstables_per_shard_factor to a very big number ?

Yes that would be the way. We could also implement a magic value like max and map it to 0 or -1 (can't use Double.MAX_VALUE as it's used in a multiplication).

blambov · 2025-07-24T12:20:55Z

Double.POSITIVE_INFINITY should be fine. If we set the flag to "1e1000", we should get infinity.

jshell> Double.parseDouble("1e1000")
$4 ==> Infinity

cbornet · 2025-07-24T12:32:38Z

Double.POSITIVE_INFINITY should be fine. If we set the flag to "1e1000", we should get infinity.

Oh right, I had fogotten about that. Means that we can also map to Double.MAX_VALUE and multiply after all.

sonarqubecloud · 2025-07-25T10:21:54Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
98.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

cassci-bot · 2025-07-25T10:28:30Z

❌ Build ds-cassandra-pr-gate/PR-1873 rejected by Butler

1 new test failure(s) in 4 builds
See build details here

Found 1 new test failures

Test	Explanation	Branch history	Upstream history
...nericOrderByTest.testOrderingAcrossManySstables	regression	🔴🔵🔵🔵	🔵🔵🔵🔵🔵🔵🔵

Found 2 known test failures

blambov · 2025-07-25T12:27:19Z

test/unit/org/apache/cassandra/db/compaction/unified/ControllerTest.java

+        Controller.validateOptions(options);
+        controller = Controller.fromOptions(cfs, options);
+        assertEquals(Controller.DEFAULT_MAX_SSTABLES_PER_SHARD_FACTOR * 10, controller.getMaxSstablesPerShardFactor(), epsilon);
+


Nit: I'd also add a test with a value that maps to infinity (e.g. 1e1000) just to make sure we can use that option.

Also consider a copy of the test where we set the factor to infinity and we end up with no compactions selected.

blambov · 2025-07-25T12:40:44Z

src/java/org/apache/cassandra/db/compaction/UnifiedCompactionStrategy.java

+            else
+            {
+                // If there are no overlaps, we look if some shards have too many SSTables.
+                // If that's the case, we perform a major compaction on those shards.


Nit: maybe add a "see CNDB-14577" as a further pointer to the reason we are doing these.

It also makes sense to extract this part in a method (I was surprised sonar isn't complaining about this, but it seems it isn't running).

cbornet · 2025-07-25T16:42:55Z

I did a test with a lot of 1-key SSTables created with this script

import os
import cassio
import uuid
import threading
import subprocess
import datetime
from cassio.config import check_resolve_session

os.environ["ASTRA_DB_APPLICATION_TOKEN"] = "xxxx"
os.environ["ASTRA_DB_SECURE_BUNDLE_PATH"] = "secure-connect-test-1873.zip"

cassio.init(auto=True)
session = check_resolve_session()

session.execute("CREATE TABLE IF NOT EXISTS default_keyspace.test_table2 (row_id text, PRIMARY KEY(row_id));")

command="curl -s  -XPOST -d '{}' http://localhost:12346/writer/api/v0/nodetool/flush/tables"

def flush(num):
    cmd = f"kubectl exec pod/cndb-writerpool-dedicated-c0466dc0-rack{num}-v0-0 -n cndb-system -- bash -c \"{command}\""
    print(cmd)
    res = subprocess.run(cmd, shell=True, capture_output=True, check=True)
    print(res)


for i in range(10000):
    print(datetime.datetime.now())
    id_ = uuid.uuid4()
    session.execute(f"INSERT INTO default_keyspace.test_table2 (row_id) VALUES ('{id_}')")

    t1 = threading.Thread(target=flush, args=(0,))
    t2 = threading.Thread(target=flush, args=(1,))
    t3 = threading.Thread(target=flush, args=(2,))

    t1.start()
    t2.start()
    t3.start()

    t1.join()
    t2.join()
    t3.join()

With the current CNDB, the SSTables are never compacted and pile up:

After applying the fix, the SSTabled are regularly compacted when the number reaches about 70:

Minutes later, the number is still limited.

cbornet · 2025-07-25T17:17:40Z

If we set a very high threshold (aka disabling the feature), eg with -Dunified_compaction.max_sstables_per_shard_factor=1e1000, SSTables start to pile up again:

cbornet marked this pull request as draft July 15, 2025 14:55

cbornet force-pushed the compact-too-many-sst branch 2 times, most recently from b12b3a2 to e85cb1d Compare July 18, 2025 12:28

blambov reviewed Jul 18, 2025

View reviewed changes

src/java/org/apache/cassandra/db/compaction/UnifiedCompactionStrategy.java Outdated Show resolved Hide resolved

src/java/org/apache/cassandra/db/compaction/UnifiedCompactionStrategy.java Outdated Show resolved Hide resolved

cbornet changed the title ~~CNDB-14577: Compact all SSTables of a level if their number reaches a…~~ CNDB-14577: Compact all SSTables of a level if their number reaches a limit Jul 18, 2025

cbornet force-pushed the compact-too-many-sst branch from ef8f14c to d1e5463 Compare July 23, 2025 12:15

blambov reviewed Jul 23, 2025

View reviewed changes

CNDB-14577: Compact all SSTables of a level shard if their number rea…

5a559a7

…ches a limit

cbornet force-pushed the compact-too-many-sst branch from d1e5463 to 7a6d9b7 Compare July 24, 2025 09:51

cbornet marked this pull request as ready for review July 24, 2025 09:52

Changes following review

bfc79cc

cbornet force-pushed the compact-too-many-sst branch from 7a6d9b7 to bfc79cc Compare July 24, 2025 09:55

cbornet changed the title ~~CNDB-14577: Compact all SSTables of a level if their number reaches a limit~~ CNDB-14577: Compact all SSTables of a shard level if their number reaches a limit Jul 24, 2025

cbornet changed the title ~~CNDB-14577: Compact all SSTables of a shard level if their number reaches a limit~~ CNDB-14577: Compact all SSTables of a level shard if their number reaches a limit Jul 24, 2025

blambov approved these changes Jul 24, 2025

View reviewed changes

Add unit tests

52fa379

cbornet force-pushed the compact-too-many-sst branch from 8295d68 to 52fa379 Compare July 25, 2025 09:36

blambov approved these changes Jul 25, 2025

View reviewed changes

blambov reviewed Jul 25, 2025

View reviewed changes

CNDB-14577: Compact all SSTables of a level shard if their number reaches a limit #1873

Are you sure you want to change the base?

CNDB-14577: Compact all SSTables of a level shard if their number reaches a limit #1873

Conversation

cbornet commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the issue

What does this PR fix and why was it fixed

Uh oh!

github-actions bot commented Jul 15, 2025 • edited by cbornet Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist before you submit for review

Uh oh!

blambov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

blambov commented Jul 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blambov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eolivelli commented Jul 24, 2025

Uh oh!

cbornet commented Jul 24, 2025

Uh oh!

blambov commented Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cbornet commented Jul 24, 2025

Uh oh!

sonarqubecloud bot commented Jul 25, 2025

Quality Gate passed

Uh oh!

cassci-bot commented Jul 25, 2025

❌ Build ds-cassandra-pr-gate/PR-1873 rejected by Butler

Found 1 new test failures

Found 2 known test failures

Uh oh!

cbornet commented Jul 15, 2025 •

edited

Loading

github-actions bot commented Jul 15, 2025 •

edited by cbornet

Loading

blambov commented Jul 24, 2025 •

edited

Loading

cbornet commented Jul 25, 2025 •

edited

Loading