CASSANALYTICS-31 SAI index support in analytics by skoppu22 · Pull Request #220 · apache/cassandra-analytics

skoppu22 · 2026-06-17T19:25:40Z

Generate SAI index files as well while producing sstable files, so no need of async rebuilding of SAI indexes after bulk write.

Circle CI : https://app.circleci.com/pipelines/github/skoppu22/cassandra-analytics/148/workflows/5eaeec43-e43b-444c-badb-ef696a3828fc

jyothsnakonisa

Some comments

jyothsnakonisa · 2026-06-24T19:11:22Z

+
+        // This builder always produces an index-less table (indexes are applied later by the 5.0 bridge), so a
+        // rebuild never carries indexes even if the caller passed index statements. buildSchema runs repeatedly per
+        // table in a JVM; if an earlier call already registered indexes, copy them onto this rebuild so it matches
+        // the registered table.
+        if (maybeExistingTableMetadata != null
+            && !maybeExistingTableMetadata.indexes.isEmpty()
+            && tableMetadata.indexes.isEmpty())
+        {
+            tableMetadata = tableMetadata.unbuild()
+                                         .indexes(maybeExistingTableMetadata.indexes)
+                                         .build();
+        }
+


RecordWriter is using 5 arg buildSchema() method which is not attaching index statements, you could eliminate copying indexes here if you use the 8 arg buildSchema() method in RecordWriter and can eventually get rid of 5 arg buildSchema() method

cassandra-analytics/cassandra-analytics-core/src/main/java/org/apache/cassandra/spark/bulkwriter/RecordWriter.java

Line 119 in 0eb3a7f

cqlTable = writerContext.bridge()

cqlTable = writerContext.bridge() .buildSchema(writerContext.schema().getTableSchema().createStatement, writerContext.job().qualifiedTableName().keyspace(), IGNORED_REPLICATION_FACTOR, writerContext.cluster().getPartitioner(), writerContext.schema().getUserDefinedTypeStatements(), null, writerContext.schema().getTableSchema().getIndexStatements(), false);

I did restructure and moved SAI specific code to extended class. Now this comment not applicable I believe

@skoppu22 This block of code is moved to FiveZeroSchemaBuilder.beforeTableRegistered but the issue still exists. You can get rid of the logic to not drop indexes in beforeTableRegistered and even the method beforeTableRegistered if you use 8 argument constructor for schemaBuilder in Recordwriter passing writerContext.schema().getTableSchema().getIndexStatements() in the constructor.

@jyothsnakonisa I have applied your recommended changes to pass index statements as part of cqlTable. We still need beforeTableRegistered. Because

Reason 1, Some builders only receive a CREATE TABLE string, with no CqlTable and no CREATE INDEX statements, so they can only build index‑less. These are public CassandraBridge APIs:

encodePartitionKeys(...) / toTokens(...) / encodePartitionKey(...) → new FiveZeroSchemaBuilder(createTableStmt, ks, rf, partitioner) readPartitionKeys(...) → same createStmt‑only builder

We can't make these pass statements without changing the public bridge API, and their callers frequently don't have the CREATE INDEX statements to pass. If one of these is the first build of a table in a JVM, the table registers without its SAI index.

Reason 2, The same table is built more than once per JVM in normal flows:

Write path, multiple partitions/executor: RecordWriter is constructed per Spark partition (mapPartitions); the 2nd+ partition on an executor rebuilds the already‑registered table. Write path, single partition: after writing, SortedSSTableWriter.validateSSTables(...) → buildLocalDataLayer → new LocalDataLayer → buildSchema rebuilds the same table in the same JVM. Read path: getCompactionScanner, getPartitionSizeIterator, rebuildBloomFilter all build from a CqlTable, repeatedly within a JVM.

Looks like there are no callers for encodePartitionKeys and readPartitionKeys. If we can remove them, then we can switch to 8 arg constructor as you mentioned, and we can simplify beforeTableRegistered just to avoid duplicate registration

Yes encodePartitionKeys & readPartitionKeys are not used anywhere in production code hence removing them should be safe. I will leave it to you whether to remove those methods or not.

jyothsnakonisa

LGTM

jyothsnakonisa · 2026-07-16T18:15:47Z

+                                      java.util.Collections.emptySet(),
+                                      java.util.Collections.emptySet());


Please remove fully qualified class names.

jyothsnakonisa · 2026-07-16T19:06:33Z

+
+        // This builder always produces an index-less table (indexes are applied later by the 5.0 bridge), so a
+        // rebuild never carries indexes even if the caller passed index statements. buildSchema runs repeatedly per
+        // table in a JVM; if an earlier call already registered indexes, copy them onto this rebuild so it matches
+        // the registered table.
+        if (maybeExistingTableMetadata != null
+            && !maybeExistingTableMetadata.indexes.isEmpty()
+            && tableMetadata.indexes.isEmpty())
+        {
+            tableMetadata = tableMetadata.unbuild()
+                                         .indexes(maybeExistingTableMetadata.indexes)
+                                         .build();
+        }
+


Yes encodePartitionKeys & readPartitionKeys are not used anywhere in production code hence removing them should be safe. I will leave it to you whether to remove those methods or not.

yifan-c

partial review; spotted a change that is needed. submitting the comments so far.

yifan-c · 2026-07-22T23:45:12Z

                fileDigests.keySet()
                           .stream()
-                           .filter(p -> p.getFileName().toString().endsWith(FileType.DATA.getFileSuffix()))
+                           .filter(SSTables::isDataComponent)


yifan-c · 2026-07-22T23:49:26Z

-                if (componentFile.getFileName().toString().endsWith("Data.db"))
+                // send the primary data component ("<descriptor>-Data.db") last; SAI per-index components
+                // such as "...+TermsData.db" are still streamed here rather than skipped
+                if (SSTables.isDataComponent(componentFile))


nit: Can you revert the comment?
This comment is already there in the javadoc of the method.
Repeating here and several other call-sites creates noise. The call-sites only need to know the method can determine whether a file is data component or not.

yifan-c · 2026-07-23T00:28:48Z

    private static final Pattern COMPACTION_STRATEGY_PATTERN = Pattern.compile("compaction\\s*=\\s*\\{\\s*'class'\\s*:\\s*'([^']+)'");

+    private static final Pattern MULTI_WHITESPACE_PATTERN = Pattern.compile("\\s+");
+    private static final Pattern SAI_USING_PATTERN = Pattern.compile("USING '([^']*\\.)?STORAGEATTACHEDINDEX'");


The pattern is not comprehensive. See the example below. cc: @maedhroz

cqlsh> DESC TABLE ks.tstsai; CREATE TABLE ks.tstsai ( id int PRIMARY KEY, val1 int, val2 int, val3 int, val4 int, val5 int ); CREATE CUSTOM INDEX val1_index ON ks.tstsai (val1) USING 'STORAGEATTACHEDINDEX'; CREATE CUSTOM INDEX val2_index ON ks.tstsai (val2) USING 'storageattachedindex'; CREATE CUSTOM INDEX val3_index ON ks.tstsai (val3) USING 'sai'; CREATE CUSTOM INDEX val4_index ON ks.tstsai (val4) USING 'StorageAttachedIndex'; CREATE CUSTOM INDEX val5_index ON ks.tstsai (val5) USING 'org.apache.cassandra.index.sai.StorageAttachedIndex';

skoppu22 and others added 8 commits March 30, 2026 20:40

draft sai support

32f2c45

first version

41020b0

pass index stmts instead of count

80cc253

Merge remote-tracking branch 'origin/trunk' into sai

bf094e7

integrate with sidecar changes

556ebe5

Merge branch 'trunk' into sai

b45b60b

fix schema

bb27eed

SAI support in restore path

d3d583d

jyothsnakonisa reviewed Jun 24, 2026

View reviewed changes

resolve comments

ef522a8

skoppu22 force-pushed the sai branch from b2311cc to 352e575 Compare July 6, 2026 17:01

apply indexes atomically

9a1ea11

skoppu22 force-pushed the sai branch from 352e575 to 9a1ea11 Compare July 7, 2026 11:12

skoppu22 added 4 commits July 7, 2026 16:56

mixed indexes handling

bca6fdb

index stmts in cqltable

d80fca9

Merge remote-tracking branch 'origin/trunk' into sai

ebaf9d7

remove index less constructors

8030786

jyothsnakonisa approved these changes Jul 16, 2026

View reviewed changes

yifan-c requested changes Jul 23, 2026

View reviewed changes

Merge branch 'trunk' into sai

83c9084

		java.util.Collections.emptySet(),
		java.util.Collections.emptySet());

Uh oh!

Conversation

skoppu22 commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jyothsnakonisa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

skoppu22 Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skoppu22 Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jyothsnakonisa 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!

yifan-c 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

skoppu22 commented Jun 17, 2026 •

edited

Loading

skoppu22 Jul 16, 2026 •

edited

Loading

skoppu22 Jul 16, 2026 •

edited

Loading