smiklosovic commented on code in PR #4282:
URL: https://github.com/apache/cassandra/pull/4282#discussion_r2250704072
##########
src/java/org/apache/cassandra/cql3/statements/schema/CreateKeyspaceStatement.java:
##########
@@ -82,6 +82,13 @@ public Keyspaces apply(ClusterMetadata metadata)
}
KeyspaceMetadata keyspaceMetadata =
KeyspaceMetadata.create(keyspaceName, attrs.asNewKeyspaceParams());
+ // we deduplicate ReplicationParams here to use the same objects in
KeyspaceMetadata
+ // as we have as keys in metadata.placements to have a fast map lookup
+ // ReplicationParams are immutable, so it is a safe optimization
+ KeyspaceParams keyspaceParams = keyspaceMetadata.params;
+ ReplicationParams replicationParams =
metadata.placements.deduplicateReplicationParams(keyspaceParams.replication);
Review Comment:
Is this logic really necessary? What we started to do is `withSwapped`
which is creating new object `new KeyspaceParams` etc ...
So one step forward but small step back as we allocate ...
What if "the deduplication" was done directly upon
`KeyspaceMetadata.create`? We do `attrs.asNewKeyspaceParams` just so we do
further transformations / deduplications on that. Why would not be
KeyspaceParams we put into `KeyspaceMetadata.create` deduplicated already?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
