CASSANALYTICS-160: Analytics should identify if a keyspace is tracked to determine appropriate stream session to use for bulk writes#214
Conversation
| } | ||
|
|
||
| @Override | ||
| public String getReplicationType() |
There was a problem hiding this comment.
CassandraClusterInfoGroup not overriding this, i.e, doesn't detect tracked keyspaces
There was a problem hiding this comment.
CassandraClusterInfoGroup is used for bulk writes through S3, for now we will not support write to tracked tables via S3. We will handle S3 code path changes in a separate JIRA
| + " AND durable_writes = true" | ||
| + " AND replication_type = 'tracked';"; | ||
| CassandraClusterInfo ci = mockClusterInfoWithSchema("mykeyspace", schema); | ||
| assertThat(ci.getReplicationType()) |
There was a problem hiding this comment.
These tests are not testing changes to CassandraClusterInfo, because MockClusterInfoForSchema overriding getReplicationType to call CqlUtils.
There was a problem hiding this comment.
Yes otherwise we need to mock sidecar client calls to get schema, avoiding that
There was a problem hiding this comment.
Can we have in-jvm tests to test CassandraClusterInfo code path then? Tests in CassandraClusterInfoTest.java should test CassandraClusterInfo. If it is difficult, we can have in-jvm tests which go through this class.
There was a problem hiding this comment.
@skoppu22 I plan to add in-jvm dtest in the follow up PR which adds the new stream session for write
There was a problem hiding this comment.
Fine, as we are merging to feature branch only
| */ | ||
| public static boolean isTracked(String replicationType) | ||
| { | ||
| return TRACKED_REPLICATION_TYPE.equalsIgnoreCase(replicationType); |
There was a problem hiding this comment.
Here we are checking for replication_type value only. But the cluster need to have mutation_tracking.enabled to true for the feature to work at Cassandra side. Can you please explore what happens in Cassandra if a keyspace was created with replication_type = tracked, but then later admins changed mutation_tracking.enabled to false? Based on these findings we may need to check for mutation_tracking.enabled as well.
There was a problem hiding this comment.
Good catch Shailaja! thanks, looks like we need to check the cluster level flag as well replication_type = tracked does not guarantee mutation tracking is enabled at cluster level
There was a problem hiding this comment.
@skoppu22 I looked into this more, in Cassandra side during imports through coordinated transfer Cassandra is only checking replication type so far, it is not checking the cluster level flag. If replication type is tracked, regardless of whether cluster level flag is enabled or disabled Cassandra takes the coordinated transfer route, we should be good to use the TrackedStreamSession.
The scenario of what if the cluster level mutation tracking flag is turned off after creating a tracked keyspace, needs further discussion and we can leave that out of this PR
skoppu22
left a comment
There was a problem hiding this comment.
Merge to feature branch is fine
… to determine appropriate stream session to use for bulk writes
dd0a789 to
9f6d7b4
Compare
No description provided.