Skip to content

Commit 4ed54ec

Browse files
krummaskrummas
committed
Improve isGossipOnlyMember and location lookup performance
Patch by marcuse; reviewed by Sam Tunnicliffe for CASSANDRA-21039
1 parent 53fff00 commit 4ed54ec

File tree

11 files changed

+234
-27
lines changed

11 files changed

+234
-27
lines changed

‎CHANGES.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
5.1
2+
* Improve isGossipOnlyMember and location lookup performance (CASSANDRA-21039)
23
* Improve debug around paused and disabled compaction (CASSANDRA-20131,CASSANDRA-19728)
34
* DiskUsageBroadcaster does not update usageInfo on node replacement (CASSANDRA-21033)
45
* Reject PrepareJoin if tokens are already assigned (CASSANDRA-21006)

‎src/java/org/apache/cassandra/config/DatabaseDescriptor.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1692,9 +1692,7 @@ public static void applySnitch()
16921692
// responsible for querying the cloud metadata service to get the public IP used for
16931693
// broadcast_address and we only want to instantiate the snitch here.
16941694
addressConfig.configureAddresses();
1695-
initializationLocator = new Locator(RegistrationStatus.instance,
1696-
FBUtilities.getBroadcastAddressAndPort(),
1697-
initialLocationProvider);
1695+
applyLocator();
16981696
nodeProximity = conf.dynamic_snitch ? new DynamicEndpointSnitch(proximity) : proximity;
16991697
localAddressReconnector = addressConfig.preferLocalConnections()
17001698
? new ReconnectableSnitchHelper(initializationLocator, true)
@@ -1709,6 +1707,14 @@ public static void applyFailureDetector()
17091707
newFailureDetector = () -> createFailureDetector(conf.failure_detector);
17101708
}
17111709

1710+
@VisibleForTesting
1711+
public static void applyLocator()
1712+
{
1713+
initializationLocator = new Locator(RegistrationStatus.instance,
1714+
FBUtilities.getBroadcastAddressAndPort(),
1715+
initialLocationProvider);
1716+
}
1717+
17121718
// definitely not safe for tools + clients - implicitly instantiates schema
17131719
public static void applyPartitioner()
17141720
{

‎src/java/org/apache/cassandra/gms/Gossiper.java‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@
106106
import static org.apache.cassandra.gms.Gossiper.GossipedWith.CMS;
107107
import static org.apache.cassandra.gms.Gossiper.GossipedWith.SEED;
108108
import static org.apache.cassandra.gms.VersionedValue.BOOTSTRAPPING_STATUS;
109+
import static org.apache.cassandra.gms.VersionedValue.HIBERNATE;
109110
import static org.apache.cassandra.gms.VersionedValue.unsafeMakeVersionedValue;
110111
import static org.apache.cassandra.net.NoPayload.noPayload;
111112
import static org.apache.cassandra.net.Verb.ECHO_REQ;
@@ -881,11 +882,15 @@ private void maybeGossipToCMS(Message<GossipDigestSyn> message)
881882
public boolean isGossipOnlyMember(InetAddressAndPort endpoint)
882883
{
883884
EndpointState epState = endpointStateMap.get(endpoint);
884-
if (epState == null)
885-
{
885+
// gossip status only used for checking transient state HIBERNATE
886+
if (epState == null || Gossiper.getGossipStatus(epState).equals(HIBERNATE))
886887
return false;
887-
}
888-
return !isDeadState(epState) && !ClusterMetadata.current().directory.allJoinedEndpoints().contains(endpoint);
888+
889+
ClusterMetadata metadata = ClusterMetadata.current();
890+
NodeId nodeId = metadata.directory.peerId(endpoint);
891+
if (nodeId == null)
892+
return false;
893+
return NodeState.isPreJoin(metadata.directory.states.get(nodeId));
889894
}
890895

891896
@VisibleForTesting

‎src/java/org/apache/cassandra/locator/Locator.java‎

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@
1818

1919
package org.apache.cassandra.locator;
2020

21+
import java.util.Map;
22+
import java.util.concurrent.ConcurrentHashMap;
23+
2124
import org.apache.cassandra.config.DatabaseDescriptor;
2225
import org.apache.cassandra.tcm.ClusterMetadata;
2326
import org.apache.cassandra.tcm.Epoch;
@@ -65,6 +68,10 @@ public class Locator
6568
// is always taken from ClusterMetadata.
6669
private volatile VersionedLocation local;
6770

71+
// Convenience to avoid making too many Directory lookups, which can be relatively expensive due to
72+
// being backed by BTreeMap/BTreeBiMap.
73+
private final Map<InetAddressAndPort, VersionedLocation> locationCache = new ConcurrentHashMap<>();
74+
6875
private static class VersionedLocation
6976
{
7077
final Epoch epoch;
@@ -167,23 +174,28 @@ private VersionedLocation versionedFromDirectory(InetAddressAndPort endpoint)
167174
return new VersionedLocation(Epoch.EMPTY, Location.UNKNOWN);
168175
source = metadata.directory;
169176
}
170-
NodeId nodeId = source.peerId(endpoint);
171-
Location location = nodeId != null ? source.location(nodeId) : Location.UNKNOWN;
172-
return new VersionedLocation(source.lastModified(), location);
177+
178+
VersionedLocation versionedLocation = locationCache.get(endpoint);
179+
if (versionedLocation == null || versionedLocation.epoch.isBefore(source.lastModified()))
180+
{
181+
NodeId nodeId = source.peerId(endpoint);
182+
if (nodeId != null)
183+
{
184+
Location location = source.location(nodeId);
185+
versionedLocation = new VersionedLocation(source.lastModified(), location);
186+
locationCache.put(endpoint, versionedLocation);
187+
}
188+
else
189+
{
190+
versionedLocation = new VersionedLocation(source.lastModified(), Location.UNKNOWN);
191+
}
192+
}
193+
return versionedLocation;
173194
}
174195

175196
private Location fromDirectory(InetAddressAndPort endpoint)
176197
{
177-
Directory source = directory;
178-
if (source == null)
179-
{
180-
ClusterMetadata metadata = ClusterMetadata.currentNullable();
181-
if (metadata == null)
182-
return Location.UNKNOWN;
183-
source = metadata.directory;
184-
}
185-
NodeId nodeId = source.peerId(endpoint);
186-
return nodeId != null ? source.location(nodeId) : Location.UNKNOWN;
198+
return versionedFromDirectory(endpoint).location;
187199
}
188200

189201
private Location initialLocation()

‎src/java/org/apache/cassandra/tcm/ClusterMetadataService.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ public static void setInstance(ClusterMetadataService newInstance)
113113
if (newInstance.metadata().myNodeId() != null)
114114
RegistrationStatus.instance.onRegistration();
115115
trace = new RuntimeException("Previously initialized trace");
116+
DatabaseDescriptor.applyLocator();
116117
}
117118

118119
@VisibleForTesting

‎src/java/org/apache/cassandra/tcm/compatibility/GossipHelper.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,10 @@ private static boolean containsDuplicateHostIds(Map<InetAddressAndPort, Endpoint
433433
Set<String> hostIds = new HashSet<>();
434434
for (EndpointState epstate : epstates.values())
435435
{
436-
String hostIdString = epstate.getApplicationState(HOST_ID).value;
436+
VersionedValue vv = epstate.getApplicationState(HOST_ID);
437+
if (vv == null)
438+
continue;
439+
String hostIdString = vv.value;
437440
if (hostIds.contains(hostIdString))
438441
return true;
439442
hostIds.add(hostIdString);

‎src/java/org/apache/cassandra/tcm/membership/NodeId.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
package org.apache.cassandra.tcm.membership;
2020

2121
import java.io.IOException;
22-
import java.util.Objects;
2322
import java.util.UUID;
2423

2524
import com.google.common.primitives.Ints;
@@ -84,13 +83,13 @@ public boolean equals(Object o)
8483
if (this == o) return true;
8584
if (o == null || getClass() != o.getClass()) return false;
8685
NodeId nodeId = (NodeId) o;
87-
return Objects.equals(id, nodeId.id);
86+
return id == nodeId.id;
8887
}
8988

9089
@Override
9190
public int hashCode()
9291
{
93-
return Objects.hash(id);
92+
return Long.hashCode(id);
9493
}
9594

9695
@Override

‎src/java/org/apache/cassandra/utils/btree/AbstractBTreeMap.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,6 @@ public Set<Map.Entry<K, V>> entrySet()
146146
public void clear() { throw new UnsupportedOperationException(); }
147147
public Map.Entry<K, V> pollFirstEntry() { throw new UnsupportedOperationException(); }
148148
public Map.Entry<K, V> pollLastEntry() { throw new UnsupportedOperationException(); }
149-
150149
protected static class KeyComparator<K, V> implements Comparator<Map.Entry<K, V>>
151150
{
152151
protected final Comparator<K> keyComparator;

‎src/java/org/apache/cassandra/utils/btree/BTreeBiMap.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,12 @@ public static <K extends Comparable<K>, V extends Comparable<V>> BTreeBiMap<K, V
6868
return BTreeBiMap.<K, V>empty(naturalOrder(), naturalOrder());
6969
}
7070

71+
private transient BTreeBiMap<V, K> inverseMap = null;
7172
@Override
7273
public BiMap<V, K> inverse()
7374
{
74-
return new BTreeBiMap<>(inverse, tree, valueComparator, asymmetricValueComparator, comparator, asymmetricComparator);
75+
BiMap<V, K> res = inverseMap;
76+
return res == null ? inverseMap = new BTreeBiMap<>(inverse, tree, valueComparator, asymmetricValueComparator, comparator, asymmetricComparator) : res;
7577
}
7678

7779
@Override
@@ -87,7 +89,7 @@ public BTreeBiMap<K, V> with(K key, V value)
8789
throw new IllegalArgumentException("Value already exists in map: " + value);
8890

8991
return new BTreeBiMap<>(BTree.update(tree, new Object[]{ entry }, comparator, UpdateFunction.noOp()),
90-
BTree.update(inverse, new Object[] { new AbstractBTreeMap.Entry<>(value, key) }, valueComparator, UpdateFunction.noOp()),
92+
BTree.update(inverse, new Object[] { inverseEntry }, valueComparator, UpdateFunction.noOp()),
9193
comparator, asymmetricComparator,
9294
valueComparator, asymmetricValueComparator);
9395
}

‎src/java/org/apache/cassandra/utils/btree/BTreeMultimap.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,25 +151,46 @@ public Set<K> keySet()
151151
return map.keySet();
152152
}
153153

154+
private transient Multiset<K> keys = null;
154155
@Override
155156
public Multiset<K> keys()
157+
{
158+
Multiset<K> res = keys;
159+
return res == null ? keys = createKeys() : res;
160+
}
161+
162+
private Multiset<K> createKeys()
156163
{
157164
ImmutableMultiset.Builder<K> keys = ImmutableMultiset.builder();
158165
keys.addAll(map.keySet());
159166
return keys.build();
160167
}
161168

169+
private transient Collection<V> values = null;
162170
@Override
163171
public Collection<V> values()
172+
{
173+
Collection<V> res = values;
174+
return res == null ? values = createValues() : res;
175+
}
176+
177+
private Collection<V> createValues()
164178
{
165179
ImmutableList.Builder<V> builder = ImmutableList.builder();
166180
for (Map.Entry<K, Collection<V>> entry : map.entrySet())
167181
builder.addAll(entry.getValue());
168182
return builder.build();
169183
}
170184

185+
private transient Collection<Map.Entry<K,V>> entries = null;
171186
@Override
172187
public Collection<Map.Entry<K, V>> entries()
188+
{
189+
Collection<Map.Entry<K, V>> res = entries;
190+
return res == null ? entries = createEntries() : res;
191+
}
192+
193+
public Collection<Map.Entry<K, V>> createEntries()
173194
{
174195
Set<Map.Entry<K, V>> entries = new HashSet<>();
175196
for (Map.Entry<K, Collection<V>> entry : map.entrySet())

0 commit comments

Comments
 (0)