Move loop out of lock in ExchangeClient::getStatus
ExchangeClient::getStatus iterates through all clients within a lock. It
is not necessary given the clients are stored in a ConcurrentMap. Also,
a jetty thread can be consumed while getting the status.
@@ -114,18 +114,23 @@ public class ExchangeClient
this.maxBufferBytes = Long.MIN_VALUE;
}
- public synchronized ExchangeClientStatus getStatus()
+ public ExchangeClientStatus getStatus()
{
+ // The stats created by this method is only for diagnostics.
+ // It does not guarantee a consistent view between different exchange clients.
+ // Guaranteeing a consistent view introduces significant lock contention.
+ ImmutableList.Builder<PageBufferClientStatus> pageBufferClientStatusBuilder = ImmutableList.builder();
+ for (HttpPageBufferClient client : allClients.values()) {
+ pageBufferClientStatusBuilder.add(client.getStatus());
+ }
+ List<PageBufferClientStatus> pageBufferClientStatus = pageBufferClientStatusBuilder.build();
+ synchronized (this) {
int bufferedPages = pageBuffer.size();
if (bufferedPages > 0 && pageBuffer.peekLast() == NO_MORE_PAGES) {
bufferedPages--;
}
-
- ImmutableList.Builder<PageBufferClientStatus> exchangeStatus = ImmutableList.builder();
- for (HttpPageBufferClient client : allClients.values()) {
- exchangeStatus.add(client.getStatus());
+ return new ExchangeClientStatus(bufferBytes, maxBufferBytes, averageBytesPerRequest, successfulRequests, bufferedPages, noMoreLocations, pageBufferClientStatus);
}
- return new ExchangeClientStatus(bufferBytes, maxBufferBytes, averageBytesPerRequest, successfulRequests, bufferedPages, noMoreLocations, exchangeStatus.build());
}
public synchronized void addLocation(URI location)
