Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor operation tracker ctor and introduce custom percentiles #1159

Merged
merged 3 commits into from
May 3, 2019
Merged

Refactor operation tracker ctor and introduce custom percentiles #1159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f14cea7
Refactor operation tracker ctor and introduce custom percentiles
jsjtzyy Apr 25, 2019
2dc44b9
address Ze's comment and clean the code
jsjtzyy Apr 26, 2019
6347c9d
introduce RouterOperation enum
jsjtzyy May 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions ambry-api/src/main/java/com.github.ambry/config/RouterConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@
*/
package com.github.ambry.config;

import com.github.ambry.utils.Utils;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;


/**
* Configuration parameters required by a {@link com.github.ambry.router.Router}.
* <p/>
Expand Down Expand Up @@ -232,6 +238,15 @@ public class RouterConfig {
@Default("false")
public final boolean routerUseGetBlobOperationForBlobInfo;

/**
* The custom percentiles of Histogram in operation tracker to be reported. This allows router to emit metrics of
* arbitrary percentiles (i.e. 97th, 93th etc). An example of this config is "0.91,0.93,0.97"(comma separated), each
* value should fall in {@code [0..1]}.
*/
@Config("router.operation.tracker.custom.percentiles")
@Default("")
public final List<Double> routerOperationTrackerCustomPercentiles;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how will these custom percentiles be used by operation tracker in the future? Right now, they are only registered in the MetricRegistry for viewing.


/**
* Create a RouterConfig instance.
* @param verifiableProperties the properties map to refer to.
Expand Down Expand Up @@ -289,5 +304,9 @@ public RouterConfig(VerifiableProperties verifiableProperties) {
verifiableProperties.getIntInRange("router.ttl.update.success.target", 2, 1, Integer.MAX_VALUE);
routerUseGetBlobOperationForBlobInfo =
verifiableProperties.getBoolean("router.use.get.blob.operation.for.blob.info", false);
List<String> customPercentiles =
Utils.splitString(verifiableProperties.getString("router.operation.tracker.custom.percentiles", ""), ",");
routerOperationTrackerCustomPercentiles =
Collections.unmodifiableList(customPercentiles.stream().map(Double::valueOf).collect(Collectors.toList()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we convert string to list when it's needed? Previous example: storeCompactionTriggers

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got you. It's OK.

}
}
140 changes: 20 additions & 120 deletions ambry-clustermap/src/main/java/com.github.ambry.clustermap/ClusterMapMetrics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,79 +67,24 @@ public ClusterMapMetrics(HardwareLayout hardwareLayout, PartitionLayout partitio

// Metrics based on HardwareLayout

this.hardwareLayoutVersion = new Gauge<Long>() {
@Override
public Long getValue() {
return getHardwareLayoutVersion();
}
};
this.partitionLayoutVersion = new Gauge<Long>() {
@Override
public Long getValue() {
return getPartitionLayoutVersion();
}
};
this.hardwareLayoutVersion = this::getHardwareLayoutVersion;
this.partitionLayoutVersion = this::getPartitionLayoutVersion;
registry.register(MetricRegistry.name(ClusterMap.class, "hardwareLayoutVersion"), hardwareLayoutVersion);
registry.register(MetricRegistry.name(ClusterMap.class, "partitionLayoutVersion"), partitionLayoutVersion);

this.datacenterCount = new Gauge<Long>() {
@Override
public Long getValue() {
return countDatacenters();
}
};
this.dataNodeCount = new Gauge<Long>() {
@Override
public Long getValue() {
return countDataNodes();
}
};
this.diskCount = new Gauge<Long>() {
@Override
public Long getValue() {
return countDisks();
}
};
this.datacenterCount = this::countDatacenters;
this.dataNodeCount = this::countDataNodes;
this.diskCount = this::countDisks;
registry.register(MetricRegistry.name(ClusterMap.class, "datacenterCount"), datacenterCount);
registry.register(MetricRegistry.name(ClusterMap.class, "dataNodeCount"), dataNodeCount);
registry.register(MetricRegistry.name(ClusterMap.class, "diskCount"), diskCount);

this.dataNodesHardUpCount = new Gauge<Long>() {
@Override
public Long getValue() {
return countDataNodesInHardState(HardwareState.AVAILABLE);
}
};
this.dataNodesHardDownCount = new Gauge<Long>() {
@Override
public Long getValue() {
return countDataNodesInHardState(HardwareState.UNAVAILABLE);
}
};
this.dataNodesUnavailableCount = new Gauge<Long>() {
@Override
public Long getValue() {
return countUnavailableDataNodes();
}
};
this.disksHardUpCount = new Gauge<Long>() {
@Override
public Long getValue() {
return countDisksInHardState(HardwareState.AVAILABLE);
}
};
this.disksHardDownCount = new Gauge<Long>() {
@Override
public Long getValue() {
return countDisksInHardState(HardwareState.UNAVAILABLE);
}
};
this.disksUnavailableCount = new Gauge<Long>() {
@Override
public Long getValue() {
return countUnavailableDisks();
}
};
this.dataNodesHardUpCount = () -> countDataNodesInHardState(HardwareState.AVAILABLE);
this.dataNodesHardDownCount = () -> countDataNodesInHardState(HardwareState.UNAVAILABLE);
this.dataNodesUnavailableCount = this::countUnavailableDataNodes;
this.disksHardUpCount = () -> countDisksInHardState(HardwareState.AVAILABLE);
this.disksHardDownCount = () -> countDisksInHardState(HardwareState.UNAVAILABLE);
this.disksUnavailableCount = this::countUnavailableDisks;
registry.register(MetricRegistry.name(ClusterMap.class, "dataNodesHardUpCount"), dataNodesHardUpCount);
registry.register(MetricRegistry.name(ClusterMap.class, "dataNodesHardDownCount"), dataNodesHardDownCount);
registry.register(MetricRegistry.name(ClusterMap.class, "dataNodesUnavailableCount"), dataNodesUnavailableCount);
Expand All @@ -149,54 +94,19 @@ public Long getValue() {

// Metrics based on PartitionLayout

this.partitionCount = new Gauge<Long>() {
@Override
public Long getValue() {
return countPartitions();
}
};
this.partitionsReadWrite = new Gauge<Long>() {
@Override
public Long getValue() {
return countPartitionsInState(PartitionState.READ_WRITE);
}
};
this.partitionsReadOnly = new Gauge<Long>() {
@Override
public Long getValue() {
return countPartitionsInState(PartitionState.READ_ONLY);
}
};
this.partitionCount = this::countPartitions;
this.partitionsReadWrite = () -> countPartitionsInState(PartitionState.READ_WRITE);
this.partitionsReadOnly = () -> countPartitionsInState(PartitionState.READ_ONLY);
registry.register(MetricRegistry.name(ClusterMap.class, "numberOfPartitions"), partitionCount);
registry.register(MetricRegistry.name(ClusterMap.class, "numberOfReadWritePartitions"), partitionsReadWrite);
registry.register(MetricRegistry.name(ClusterMap.class, "numberOfReadOnlyPartitions"), partitionsReadOnly);

this.isMajorityReplicasDown = new Gauge<Boolean>() {
@Override
public Boolean getValue() {
return isMajorityOfReplicasDown();
}
};
this.isMajorityReplicasDown = this::isMajorityOfReplicasDown;
registry.register(MetricRegistry.name(ClusterMap.class, "isMajorityReplicasDown"), isMajorityReplicasDown);

this.rawCapacityInBytes = new Gauge<Long>() {
@Override
public Long getValue() {
return getRawCapacity();
}
};
this.allocatedRawCapacityInBytes = new Gauge<Long>() {
@Override
public Long getValue() {
return getAllocatedRawCapacity();
}
};
this.allocatedUsableCapacityInBytes = new Gauge<Long>() {
@Override
public Long getValue() {
return getAllocatedUsableCapacity();
}
};
this.rawCapacityInBytes = this::getRawCapacity;
this.allocatedRawCapacityInBytes = this::getAllocatedRawCapacity;
this.allocatedUsableCapacityInBytes = this::getAllocatedUsableCapacity;
registry.register(MetricRegistry.name(ClusterMap.class, "rawCapacityInBytes"), rawCapacityInBytes);
registry.register(MetricRegistry.name(ClusterMap.class, "allocatedRawCapacityInBytes"),
allocatedRawCapacityInBytes);
Expand All @@ -218,12 +128,7 @@ public Long getValue() {

private void addDataNodeToStateMetrics(final DataNode dataNode) {
final String metricName = dataNode.getHostname() + "-" + dataNode.getPort() + "-ResourceState";
Gauge<Long> dataNodeState = new Gauge<Long>() {
@Override
public Long getValue() {
return dataNode.getState() == HardwareState.AVAILABLE ? 1L : 0L;
}
};
Gauge<Long> dataNodeState = () -> dataNode.getState() == HardwareState.AVAILABLE ? 1L : 0L;
registry.register(MetricRegistry.name(ClusterMap.class, metricName), dataNodeState);
dataNodeStateList.add(dataNodeState);
}
Expand All @@ -232,12 +137,7 @@ private void addDiskToStateMetrics(final Disk disk) {
final String metricName =
disk.getDataNode().getHostname() + "-" + disk.getDataNode().getPort() + "-" + disk.getMountPath()
+ "-ResourceState";
Gauge<Long> diskState = new Gauge<Long>() {
@Override
public Long getValue() {
return disk.getState() == HardwareState.AVAILABLE ? 1L : 0L;
}
};
Gauge<Long> diskState = () -> disk.getState() == HardwareState.AVAILABLE ? 1L : 0L;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for cleaning up this code!

registry.register(MetricRegistry.name(ClusterMap.class, metricName), diskState);
dataNodeStateList.add(diskState);
}
Expand Down
59 changes: 16 additions & 43 deletions ambry-network/src/main/java/com.github.ambry.network/BlockingChannelConnectionPool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,32 +73,17 @@ public BlockingChannelInfo(ConnectionPoolConfig config, String host, Port port,
this.sslSocketFactory = sslSocketFactory;
this.sslConfig = sslConfig;

availableConnections = new Gauge<Integer>() {
@Override
public Integer getValue() {
return blockingChannelAvailableConnections.size();
}
};
availableConnections = blockingChannelAvailableConnections::size;
registry.register(
MetricRegistry.name(BlockingChannelInfo.class, host + "-" + port.getPort() + "-availableConnections"),
availableConnections);

activeConnections = new Gauge<Integer>() {
@Override
public Integer getValue() {
return blockingChannelActiveConnections.size();
}
};
activeConnections = blockingChannelActiveConnections::size;
registry.register(
MetricRegistry.name(BlockingChannelInfo.class, host + "-" + port.getPort() + "-activeConnections"),
activeConnections);

totalNumberOfConnections = new Gauge<Integer>() {
@Override
public Integer getValue() {
return numberOfConnections.intValue();
}
};
totalNumberOfConnections = numberOfConnections::intValue;
registry.register(
MetricRegistry.name(BlockingChannelInfo.class, host + "-" + port.getPort() + "-totalNumberOfConnections"),
totalNumberOfConnections);
Expand Down Expand Up @@ -240,8 +225,7 @@ public void destroyBlockingChannel(BlockingChannel blockingChannel) {
}

/**
* Returns the number of connections with this BlockingChannelInfo
* @return
* @return the number of connections with this BlockingChannelInfo
*/
public int getNumberOfConnections() {
return this.numberOfConnections.intValue();
Expand Down Expand Up @@ -310,40 +294,29 @@ public BlockingChannelConnectionPool(ConnectionPoolConfig config, SSLConfig sslC
connectionDestroyTime =
registry.timer(MetricRegistry.name(BlockingChannelConnectionPool.class, "connectionDestroyTime"));

totalNumberOfNodesConnectedTo = new Gauge<Integer>() {
@Override
public Integer getValue() {
int noOfNodesConnectedTo = 0;
for (BlockingChannelInfo blockingChannelInfo : connections.values()) {
if (blockingChannelInfo.getNumberOfConnections() > 0) {
noOfNodesConnectedTo++;
}
totalNumberOfNodesConnectedTo = () -> {
int noOfNodesConnectedTo = 0;
for (BlockingChannelInfo blockingChannelInfo : connections.values()) {
if (blockingChannelInfo.getNumberOfConnections() > 0) {
noOfNodesConnectedTo++;
}
return noOfNodesConnectedTo;
}
return noOfNodesConnectedTo;
};
registry.register(MetricRegistry.name(BlockingChannelConnectionPool.class, "totalNumberOfNodesConnectedTo"),
totalNumberOfNodesConnectedTo);

totalNumberOfConnections = new Gauge<Integer>() {
@Override
public Integer getValue() {
int noOfConnections = 0;
for (BlockingChannelInfo blockingChannelInfo : connections.values()) {
noOfConnections += blockingChannelInfo.getNumberOfConnections();
}
return noOfConnections;
totalNumberOfConnections = () -> {
int noOfConnections = 0;
for (BlockingChannelInfo blockingChannelInfo : connections.values()) {
noOfConnections += blockingChannelInfo.getNumberOfConnections();
}
return noOfConnections;
};
registry.register(MetricRegistry.name(BlockingChannelConnectionPool.class, "totalNumberOfConnections"),
totalNumberOfConnections);
requestsWaitingToCheckoutConnectionCount = new AtomicInteger(0);
requestsWaitingToCheckoutConnection = new Gauge<Integer>() {
@Override
public Integer getValue() {
return requestsWaitingToCheckoutConnectionCount.get();
}
};
requestsWaitingToCheckoutConnection = requestsWaitingToCheckoutConnectionCount::get;
registry.register(MetricRegistry.name(BlockingChannelConnectionPool.class, "requestsWaitingToCheckoutConnection"),
requestsWaitingToCheckoutConnection);
sslSocketFactoryClientInitializationCount = registry.counter(
Expand Down
Loading