CASSANDRA-21105 CEP-58: Add satellite replication strategy stub by bdeggleston · Pull Request #4652 · apache/cassandra

bdeggleston · 2026-03-03T19:42:38Z

No description provided.

src/java/org/apache/cassandra/locator/ReplicationFactor.java

src/java/org/apache/cassandra/locator/SatelliteDatacenterReplicationStrategy.java

maedhroz · 2026-03-12T20:42:56Z

src/java/org/apache/cassandra/locator/SatelliteReplicationStrategy.java

+                return false;
+
+            if (primary != null)
+                cfgEx("'primary' option specified multiple times");


Superficially, test coverage looks pretty good. The only paths that aren't hit are the validation failures like this one, the similar things in validateExpectedOptions(), and client warnings in maybeWarnOnOptions(). We could test those explicitly if you think it would be valuable...

src/java/org/apache/cassandra/locator/SatelliteDatacenterReplicationStrategy.java

maedhroz · 2026-03-12T22:35:14Z

src/java/org/apache/cassandra/locator/SatelliteReplicationStrategy.java

+
+        ReplicaGroups built = builder.build();
+        return new DataPlacement(built, built);
+    }


I think the most pressing design question in my head here is whether there's more we can do to deduplicate with respect to the NTS stuff that already exists. I can see why you might not want to directly extend NetworkTopologyStrategy given how its constructor assigns datacenters and aggregateRf, but if we could share those bits of state, it seems like calculateNaturalReplicas(), calculateDataPlacement(), getReplicationFactor(), etc. could be shared.

maybeWarnOnOptions() has enough subtle differences that it might not be sharable directly, but I'd say the RF to node count comparison stuff might be.

AbstractNetworkTopologyStrategy kind of sucks as a name...maybe DatacenterAwareReplicationStrategy? That would at least capture the major shared detail...

Yes, there definitely is. However I wanted to avoid touching NTS if possible. I think that minimizing risk for non-SRS use cases is preferable to a cleaner class structure. But we can discuss if you feel strongly about it

The main thing is calculateDataPlacement(). It's just annoying to dedupe without inheritance, because it calls calculateNaturalReplicas(). TCM extracted the static calculateNaturalReplicas(), which we reuse here at least. It might be past the boundary of silliness to make a static calculateDataPlacement() in NTS that has to like take a lambda for calculateNaturalReplicas() :\

yeah but there's not really a lot going on in calculated data placement. It basically calls calculateNaturalReplicas for each range that's passed in

maedhroz · 2026-03-12T22:42:41Z

src/java/org/apache/cassandra/locator/SatelliteDatacenterReplicationStrategy.java

+ *
+ * Uses NetworkTopologyStrategy replica selection algorithm for compatibility with existing NTS keyspaces
+ */
+public class SatelliteDatacenterReplicationStrategy extends AbstractReplicationStrategy


Let's bike-shed on this name, since it's going to be publicly facing, right? Alternatives:

SatelliteReplicationStrategy - Shorter, but would you get the impression it was only for satellite DCs themselves?

SatelliteTopologyStrategy - Going for a "topology that contains satellites" vibe

maedhroz · 2026-03-12T22:45:28Z

src/java/org/apache/cassandra/locator/SatelliteReplicationStrategy.java

+        this.primaryDC = opts.primary;
+        this.disabledDCs = Collections.unmodifiableSet(opts.disabledDCs);
+
+        validate();


What if we moved this validate() method to the StrategyOptions class itself, given that all operates over fields in that class anyway? We could also move that opts.validate() call before the assignments above.

I wanted something that validated the final state of the replication strategy itself, not it's builder input

see #4652 (comment)

Even if we don't do that ^, fair to say the state is exactly the same, with the exception of some things being wrapped in unmodifiable collections?

src/java/org/apache/cassandra/locator/SatelliteDatacenterReplicationStrategy.java

maedhroz · 2026-03-12T22:51:45Z

src/java/org/apache/cassandra/locator/SatelliteReplicationStrategy.java

+
+            allDatacenters.putAll(fullDatacenters);
+            satellites.forEach((dc, info) -> allDatacenters.put(dc, info.rf));
+        }


nit: What if we just had a StrategyOptions.fromConfigMap() or something that just fed immutable stuff to the actual StrategyOptions constructor. Not sure if we really care about creating StrategyOptions directly for tests or whatever, but that would be easier too...

i.e. StrategyOptions would have final fields and could even be used directly as a field for the strategy class itself?

then we'd basically be back at the initial massive constructor that motivated me to collate everything in a mutable StrategyOptions class in the first place, albeit in a static method instead of the strategy ctor. I'm not opposed to restructuring strategy construction, this just seemed like the cleanest way to parse, organize, and validate the different strategy components before locking them into immutable collections and validating the final configuration. I think if we were to restructure this, the next best option would just be to put everything into the strategy ctor itself. WDYT?

maedhroz · 2026-03-12T22:54:28Z

src/java/org/apache/cassandra/locator/SatelliteReplicationStrategy.java

+            String satelliteName = parts[1];
+
+            if (parentDc.contains(".") || satelliteName.contains("."))
+                cfgEx("Datacenter names cannot contain dots: '%s'", key);


nit: Message is duplicated on 243

maedhroz · 2026-03-12T22:55:04Z

src/java/org/apache/cassandra/locator/SatelliteReplicationStrategy.java

+
+            String[] rfParts = value.split("/");
+            if (rfParts.length != 2)
+                cfgEx("Invalid replication factor format for satellite '%s': '%s'", satelliteName, value);


nit: Message duplicated on 229

I'd like to leave this (and the other duplicate message) as is if you don't mind. They're not significant duplications, and I'm not sure they'd be worth the effort to dedupe

maedhroz

Left a bunch of comments that are mostly structural in nature and not correctness-related. The flat configuration structure is fine with me, but I don't know if we need to vet it elsewhere.

Patch by Blake Eggleston; Reviewed by Caleb Rackliffe for CASSANDRA-21105

bdeggleston requested a review from maedhroz March 3, 2026 19:42