CASSANALYTICS-194: Parse <replicas>/<transient> replication factor fo… - #238
Open
mansikhara wants to merge 6 commits into
Open
mansikhara wants to merge 6 commits into
mansikhara wants to merge 6 commits into
Conversation
… keyspace for bulk writes (apache#214) patch by Saranya Krishnakumar; reviewed by Shailaja Koppu, Yifan Cai for CASSANALYTICS-160
…r witness-enabled keyspaces Cassandra accepts a replication factor of the form <replicas>/<transient>, which witness replicas under mutation tracking reuse, so a witness-enabled keyspace declares 'datacenter1': '3/1' meaning three replicas of which one is a witness. CqlUtils.extractReplicationFactor() passed each datacenter value to Integer.parseInt, so a bulk read against any such keyspace failed during job setup with an uncaught NumberFormatException. Transient counts are now tracked per datacenter alongside the existing totals and exposed through getFullReplicationFactor(), getTransientReplicationFactor(), getFullReplicas(dc), getTransientReplicas(dc) and hasTransientReplicas(). getTotalReplicationFactor() keeps its existing meaning of all replicas including witnesses, so behaviour for untracked keyspaces is unchanged. Parsing applies the same constraints Cassandra enforces in locator.ReplicationFactor.validate. A new parseStrict factory reports an unparseable or empty replication map at parse time rather than dropping the datacenter and failing later with a misleading "DC not found in replication factor"; the lenient constructor is retained unchanged for CDC callers. The Kryo serializer and CassandraRing's hand-rolled JDK readObject/writeObject are updated so the new field survives serialization to Spark executors. CassandraRing gains a pinned serialVersionUID and a format-version byte, because the class signature did not change and an older stream would otherwise be silently misread rather than rejected. Prerequisite for CASSANALYTICS-164. patch by Mansi Khara; reviewed by TBD for CASSANALYTICS-194
sarankk
reviewed
Sep 15, 2026
sarankk
left a comment
sarankk
left a comment
Contributor
There was a problem hiding this comment.
Thanks Mansi, left few comments
…ictly
Combines the per-datacenter total and transient counts into a single Map<String, ReplicaCounts>
instead of two parallel maps. The maps could previously drift apart, since nothing tied
options={dc1=3} to transientOptions={dc1=1}; holding them together makes that unrepresentable.
ReplicaCounts becomes a public nested type exposing allReplicas, fullReplicas and
transientReplicas. getOptions() and getTransientOptions() are retained, now derived and
unmodifiable, so CassandraRing, ConsistencyLevel and PartitionedDataLayer are unaffected.
Removes the lenient parse. Its only two callers built hardcoded maps that cannot fail to parse,
so it was never needed, and silently dropping a datacenter that Cassandra itself would reject is
the wrong default: 3/3 and a negative replication factor now raise rather than being skipped.
parseStrict therefore disappears as a separate factory, since strict is the only behaviour.
Corrects the javadoc: transient replication predates mutation tracking, and it is witness
replicas (CEP-46) that reuse the form. Drops replication_type from the replication factor test
fixtures, since Sidecar builds its schema response from the driver's exportAsString() which does
not emit that property, and it is irrelevant to replication factor parsing.
mansikhara
force-pushed
the
mutation-tracking-support
branch
from
September 15, 2026 20:45
e2d75d0 to
3a07399
Compare
sarankk
reviewed
Sep 16, 2026
sarankk
left a comment
sarankk
left a comment
Contributor
There was a problem hiding this comment.
Thanks Mansi!, few more comments.
Restructures the constructors so each one resolves its input to per-datacenter counts and delegates to a single canonical constructor, rather than repeating the class lookup, the class-key skip and the empty check. Parsing raw strings and merging separate total and transient maps move into named helpers. Removes the validate flag from the private constructor. It was only ever passed false, so the guarded branch was dead: the flag was left over from an earlier version of readResolve that reconstructed the instance instead of rejecting it. CassandraRing now writes the ReplicationFactor as an object rather than destructuring it into strategy, options and transient options. ReplicationFactor is Serializable, so this is both shorter and independent of how it represents per-datacenter counts, which is what made the previous version need updating in the first place. Drops the explanatory comment from the replication factor test fixture.
sarankk
reviewed
Sep 17, 2026
…center consistently The aggregate counts and the two derived per-datacenter maps were recomputed on every call, and getOptions in particular is called in a loop from CassandraRing.init. They are now computed once in the canonical constructor, which this class can do because it is immutable. The derived fields are transient, so they are absent from the serialized form rather than duplicating what replication already holds. readResolve rebuilds through the canonical constructor, and the Kryo serializer already went through it, so neither path can leave them out of step. Tests cover both round trips. getTransientReplicas returned 0 for a datacenter with no replication factor while getFullReplicas threw for the same input. That conflated "no transient replicas" with "not a datacenter of this keyspace" and hid the second case. It now throws, matching getFullReplicas. No production caller passes an unknown datacenter.
sarankk
force-pushed
the
mutation-tracking-support
branch
from
September 18, 2026 22:46
6064e02 to
b0c400b
Compare
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.
…r witness-enabled keyspaces
Cassandra accepts a replication factor of the form /, which witness replicas under mutation tracking reuse, so a witness-enabled keyspace declares 'datacenter1': '3/1' meaning three replicas of which one is a witness.
CqlUtils.extractReplicationFactor() passed each datacenter value to Integer.parseInt, so a bulk read against any such keyspace failed during job setup with an uncaught NumberFormatException.
Transient counts are now tracked per datacenter alongside the existing totals and exposed through getFullReplicationFactor(), getTransientReplicationFactor(), getFullReplicas(dc), getTransientReplicas(dc) and hasTransientReplicas(). getTotalReplicationFactor() keeps its existing meaning of all replicas including witnesses, so behaviour for untracked keyspaces is unchanged. Parsing applies the same constraints Cassandra enforces in locator.ReplicationFactor.validate. A new parseStrict factory reports an unparseable or empty replication map at parse time rather than dropping the datacenter and failing later with a misleading "DC not found in replication factor"; the lenient constructor is retained unchanged for CDC callers. The Kryo serializer and CassandraRing's hand-rolled JDK readObject/writeObject are updated so the new field survives serialization to Spark executors.
Prerequisite for CASSANALYTICS-164.