CASSANALYTICS-31 SAI index support in analytics#220
Open
skoppu22 wants to merge 11 commits into
Open
Conversation
Comment on lines
+288
to
+295
| CqlTable cqlTable = new SchemaBuilder(createStatement, keyspace, replicationFactor, partitioner, | ||
| cassandraTypes -> udts, tableId, indexStatements, enableCdc).build(); | ||
| // SAI is a Cassandra 5.0+ feature: register any Storage Attached Index definitions on the table metadata | ||
| // after the shared SchemaBuilder has built/registered the (index-less) table. | ||
| // buildSchema is invoked repeatedly within a JVM (the index-carrying per-partition RecordWriter included), | ||
| // so this re-applies the SAI definitions whenever a rebuild left the table without them. | ||
| StorageAttachedIndexApplier.maybeApply(keyspace, cqlTable.table(), indexStatements); | ||
| return cqlTable; |
Contributor
There was a problem hiding this comment.
Schema Is created first and then SAI indexes are attached in the next step, I wondering about the brief moment between two statements where the schema doesn't have SAI indexes.
WDYT about merging both operations into a single CassandraSchema.apply in the 5.0 bridge? you might have to override CassandraSchema.apply call entirely from the 5.0 bridge and don't use SchemaBuilder's constructor at all for the schema update.
I am open to other approaches too
Contributor
Author
There was a problem hiding this comment.
Done using a afterRegister function
Comment on lines
+227
to
+240
|
|
||
| // 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(); | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
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
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);
Contributor
Author
There was a problem hiding this comment.
I did restructure and moved SAI specific code to extended class. Now this comment not applicable I believe
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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