netudima commented on code in PR #4282:
URL: https://github.com/apache/cassandra/pull/4282#discussion_r2252773571
##########
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:
> Why would not we put KeyspaceParams into KeyspaceMetadata.create
deduplicated already?
Yes, agree, it will be more clear and will reduce amount of juggling with
the objects (so, the logic will be more readable).
> So one step forward but also some step back as we allocate ...
Just to clarify: this method is invoked when we apply schema changes or when
we load schema on startup (from TCM log), so this method is not on a hot path
and I would not worry a lot about allocation of several extra objects here.
The goal of this deduplication is not to reduce memory usage but to make
lookup from metadata.placements
(`metadata.placements.get(ks.params.replication)`) more efficient within a hot
path during a plain write.
ReplicationParams is a key for metadata.placements map, so when we do a
lookup from it we have to compare ReplicationParams object provided as a key to
the get operation and a ReplicationParams object stored in the map.
Deduplication utilises a fast path within this equals via == instead of full
comparison of inner structures (which is much more expensive).
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
