Skip to content

Commit ab95a68

Browse files
committed
Optimize DataPlacement lookup by ReplicationParams
Avoid double lookup of the same DataPlacement in forNonLocalStrategyTokenRead and forNonLocalStrategyTokenWrite methods Memorize hashCode value Deduplicate ReplicationParams to use the same objects in DataPlacements and KeyspaceMetadata to use the fast == path in the equals Patch by Dmitry Konstantinov; reviewed by TBD for CASSANDRA-20804
1 parent 5aeaef5 commit ab95a68

File tree

5 files changed

+43
-5
lines changed

5 files changed

+43
-5
lines changed

src/java/org/apache/cassandra/cql3/statements/schema/CreateKeyspaceStatement.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,13 @@ public Keyspaces apply(ClusterMetadata metadata)
8282
}
8383

8484
KeyspaceMetadata keyspaceMetadata = KeyspaceMetadata.create(keyspaceName, attrs.asNewKeyspaceParams());
85+
// we deduplicate ReplicationParams here to use the same objects in KeyspaceMetadata
86+
// as we have as keys in metadata.placements to have a fast map lookup
87+
// ReplicationParams are immutable, so it is a safe optimization
88+
KeyspaceParams keyspaceParams = keyspaceMetadata.params;
89+
ReplicationParams replicationParams = metadata.placements.deduplicateReplicationParams(keyspaceParams.replication);
90+
keyspaceParams = keyspaceParams.withSwapped(replicationParams);
91+
keyspaceMetadata = keyspaceMetadata.withSwapped(keyspaceParams);
8592

8693
if (keyspaceMetadata.params.replication.klass.equals(LocalStrategy.class))
8794
throw ire("Unable to use given strategy class: LocalStrategy is reserved for internal use.");

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

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import org.apache.cassandra.schema.TableId;
3232
import org.apache.cassandra.service.reads.ReadCoordinator;
3333
import org.apache.cassandra.tcm.ClusterMetadata;
34+
import org.apache.cassandra.tcm.ownership.DataPlacement;
3435
import org.apache.cassandra.utils.FBUtilities;
3536

3637
import java.util.Set;
@@ -238,8 +239,9 @@ public static ReplicaLayout.ForTokenWrite forTokenWriteLiveAndDown(ClusterMetada
238239
{
239240
// todo deduplicate so that "pending" contains "read - write",
240241
// which is a hack until we revisit how consistency level handles pending
241-
natural = forNonLocalStrategyTokenRead(metadata, ks, token);
242-
pending = forNonLocalStrategyTokenWrite(metadata, ks, token).without(natural.endpoints());
242+
DataPlacement dataPlacement = metadata.placements.get(ks.params.replication);
243+
natural = forNonLocalStrategyTokenRead(dataPlacement, token);
244+
pending = forNonLocalStrategyTokenWrite(dataPlacement, token).without(natural.endpoints());
243245
}
244246
return forTokenWrite(replicationStrategy, natural, pending);
245247
}
@@ -392,14 +394,25 @@ static EndpointsForRange forNonLocalStategyRangeRead(ClusterMetadata metadata, K
392394

393395
public static EndpointsForToken forNonLocalStrategyTokenRead(ClusterMetadata metadata, KeyspaceMetadata keyspace, Token token)
394396
{
395-
return metadata.placements.get(keyspace.params.replication).reads.forToken(token).get();
397+
return forNonLocalStrategyTokenRead(metadata.placements.get(keyspace.params.replication), token);
398+
}
399+
400+
public static EndpointsForToken forNonLocalStrategyTokenRead(DataPlacement dataPlacement, Token token)
401+
{
402+
return dataPlacement.reads.forToken(token).get();
396403
}
397404

398405
static EndpointsForToken forNonLocalStrategyTokenWrite(ClusterMetadata metadata, KeyspaceMetadata keyspace, Token token)
399406
{
400-
return metadata.placements.get(keyspace.params.replication).writes.forToken(token).get();
407+
return forNonLocalStrategyTokenWrite(metadata.placements.get(keyspace.params.replication), token);
401408
}
402409

410+
static EndpointsForToken forNonLocalStrategyTokenWrite(DataPlacement dataPlacement, Token token)
411+
{
412+
return dataPlacement.writes.forToken(token).get();
413+
}
414+
415+
403416
static EndpointsForRange forLocalStrategyRange(ClusterMetadata metadata, AbstractReplicationStrategy replicationStrategy, AbstractBounds<PartitionPosition> range)
404417
{
405418
return replicationStrategy.calculateNaturalReplicas(range.right.getToken(), metadata);

src/java/org/apache/cassandra/schema/KeyspaceParams.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,11 @@ public static KeyspaceParams nts(Object... args)
117117
return new KeyspaceParams(true, ReplicationParams.nts(args), FastPathStrategy.simple());
118118
}
119119

120+
public KeyspaceParams withSwapped(ReplicationParams params)
121+
{
122+
return new KeyspaceParams(durableWrites, params, fastPath);
123+
}
124+
120125
public void validate(String name, ClientState state, ClusterMetadata metadata)
121126
{
122127
replication.validate(name, state, metadata);

src/java/org/apache/cassandra/schema/ReplicationParams.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,13 @@ public final class ReplicationParams
5757

5858
public final Class<? extends AbstractReplicationStrategy> klass;
5959
public final ImmutableMap<String, String> options;
60+
private final int hashCode;
6061

6162
private ReplicationParams(Class<? extends AbstractReplicationStrategy> klass, Map<String, String> options)
6263
{
6364
this.klass = klass;
6465
this.options = ImmutableMap.copyOf(options);
66+
this.hashCode = Objects.hashCode(this.klass, this.options);
6567
}
6668

6769
@VisibleForTesting
@@ -234,7 +236,7 @@ public boolean equals(Object o)
234236
@Override
235237
public int hashCode()
236238
{
237-
return Objects.hashCode(klass, options);
239+
return hashCode;
238240
}
239241

240242
@Override

src/java/org/apache/cassandra/tcm/ownership/DataPlacements.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,17 @@ public boolean equivalentTo(DataPlacements other)
159159
.allMatch(e -> e.getValue().equivalentTo(other.get(e.getKey())));
160160
}
161161

162+
public ReplicationParams deduplicateReplicationParams(ReplicationParams replicationParams)
163+
{
164+
if (this.get(replicationParams) == null)
165+
return replicationParams;
166+
167+
for (ReplicationParams placementReplicationParams : keys())
168+
if (placementReplicationParams.equals(replicationParams))
169+
return placementReplicationParams;
170+
return replicationParams;
171+
}
172+
162173
public static DataPlacements sortReplicaGroups(DataPlacements placements, Comparator<Replica> comparator)
163174
{
164175
Builder builder = DataPlacements.builder(placements.size());

0 commit comments

Comments
 (0)