Skip to content

HDDS-9522. Add percentile for ProtocolMessageMetrics #5479

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

Closed
YuanbenWang wants to merge 6 commits into from
Closed

HDDS-9522. Add percentile for ProtocolMessageMetrics #5479

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e520901
HDDS-9522. Add percentile for ProtocolMessageMetrics
Oct 23, 2023
b194f41
change the unit of time
Nov 10, 2023
810d2ea
fix checkstyle
Nov 14, 2023
e8df67c
fix metrics name
Nov 16, 2023
7f83f36
cleanup useless method
Nov 29, 2023
e40b747
put quantiles back after adding
Jan 2, 2024
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
3 changes: 3 additions & 0 deletions hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,9 @@ public final class OzoneConfigKeys {
public static final int OZONE_CLIENT_EC_GRPC_RETRIES_MAX_DEFAULT = 3;
public static final String OZONE_GPRC_METRICS_PERCENTILES_INTERVALS_KEY
= "ozone.grpc.metrics.percentiles.intervals";
public static final String
OZONE_PROTOCOL_MESSAGE_METRICS_PERCENTILES_INTERVALS
= "ozone.protocol.message.metrics.percentiles.intervals";

/**
* Ozone administrator users delimited by comma.
Expand Down
5 changes: 3 additions & 2 deletions ...r-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,11 @@ public HddsDispatcher(ConfigurationSource config, ContainerSet contSet,
: new NoopTokenVerifier();

protocolMetrics =
new ProtocolMessageMetrics<>(
ProtocolMessageMetrics.create(
"HddsDispatcher",
"HDDS dispatcher metrics",
Type.values());
Type.values(),
conf);

this.dispatcher =
new OzoneProtocolMessageDispatcher<>("DatanodeClient",
Expand Down
50 changes: 46 additions & 4 deletions hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/ProtocolMessageMetrics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,17 @@
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong;

import org.apache.hadoop.hdds.conf.ConfigurationSource;
import org.apache.hadoop.metrics2.MetricsCollector;
import org.apache.hadoop.metrics2.MetricsInfo;
import org.apache.hadoop.metrics2.MetricsRecordBuilder;
import org.apache.hadoop.metrics2.MetricsSource;
import org.apache.hadoop.metrics2.MetricsTag;
import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem;
import org.apache.hadoop.metrics2.lib.Interns;
import org.apache.hadoop.metrics2.lib.MetricsRegistry;
import org.apache.hadoop.metrics2.lib.MutableQuantiles;
import org.apache.hadoop.ozone.OzoneConfigKeys;
import org.apache.ratis.util.UncheckedAutoCloseable;

/**
Expand All @@ -40,32 +44,59 @@ public class ProtocolMessageMetrics<KEY> implements MetricsSource {

private final String description;

private final MetricsRegistry registry;

private final boolean quantileEnable;

private final Map<KEY, AtomicLong> counters =
new ConcurrentHashMap<>();

private final Map<KEY, AtomicLong> elapsedTimes =
new ConcurrentHashMap<>();

private final Map<KEY, MutableQuantiles[]> quantiles =
new ConcurrentHashMap<>();

private final AtomicInteger concurrency = new AtomicInteger(0);

public static <KEY> ProtocolMessageMetrics<KEY> create(String name,
String description, KEY[] types) {
return new ProtocolMessageMetrics<KEY>(name, description, types);
String description, KEY[] types, ConfigurationSource conf) {
return new ProtocolMessageMetrics<KEY>(name, description, types, conf);
}

public ProtocolMessageMetrics(String name, String description,
Comment thread
xBis7 marked this conversation as resolved.
Outdated
KEY[] values) {
KEY[] values, ConfigurationSource conf) {
this.name = name;
this.description = description;
registry = new MetricsRegistry(name + "MessageMetrics");
int[] intervals = conf.getInts(
OzoneConfigKeys.OZONE_PROTOCOL_MESSAGE_METRICS_PERCENTILES_INTERVALS);
quantileEnable = (intervals.length > 0);
for (KEY value : values) {
counters.put(value, new AtomicLong(0));
elapsedTimes.put(value, new AtomicLong(0));
if (quantileEnable) {
MutableQuantiles[] mutableQuantiles =
new MutableQuantiles[intervals.length];
quantiles.put(value, mutableQuantiles);
for (int i = 0; i < intervals.length; i++) {
mutableQuantiles[i] = registry.newQuantiles(
value.toString() + "RpcTime" + intervals[i] + "s",
value.toString() + "rpc time in milli second",
"ops", "latency", intervals[i]);

@xichen01 xichen01 Nov 8, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It would be better to clarify the unit of time in the name of the Metrics, such as Use latencyMS

@YuanbenWang YuanbenWang Nov 13, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Now the name of the Metric will be like “InfoVolumeRpcTime127s90thPercentileLatency” and the unit of time is including in it as "s". It will be appreciated if you could tell me how to let it more suitable. How about "InfoVolumeRpcTime127latencySeconds90thPercentileLatency"? @xichen01

@xichen01 xichen01 Nov 14, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think that InfoVolumeRpcTime127s90thPercentileLatencyMs is available, the 127s is the time of sampling, Ms is the unit of the Metrics value.

@YuanbenWang YuanbenWang Nov 14, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

InfoVolumeRpcTime127latencySeconds90thPercentileLatency

Thank you for your feedback! I appreciate your clarification on the suggestion you provided. I now understand it better, and I will proceed to make the necessary modifications according to your advice.

@xichen01 xichen01 Nov 15, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry for not expressing it clearly, I think we can remove the type from the Metrics name when we move it to the tag, so the name can be 127s90thPercentileLatencyMs

Suggested change
"ops", "latency", intervals[i]);
for (KEY value : values) {
MetricsRegistry registry = new MetricsRegistry(name + "MessageMetrics");
registries.put(value, registry);
//...
if (quantileEnable) {
mutableQuantiles[i] = registry.newQuantiles(
"_" + intervals[i] + "s_",
value.toString() + "rpc time in milli second",
"ops", "latencyMS", intervals[i]);

The Metrics name will be like this om_client_protocol_127s_99th_percentile_latency_ms{type="TransferLeadership",hostname="xxx"} 0 om_client_protocol_127s_num_ops{type="TransferLeadership",hostname="xxx"}
This name is consistent with the original Metrics style and is simpler.

}
}
}
}

public void increment(KEY key, long duration) {
counters.get(key).incrementAndGet();
elapsedTimes.get(key).addAndGet(duration);
if (quantileEnable) {
for (MutableQuantiles q : quantiles.get(key)) {
Comment thread
xBis7 marked this conversation as resolved.
Outdated
q.add(duration);
}
}
}

public UncheckedAutoCloseable measure(KEY key) {
Expand All @@ -74,7 +105,13 @@ public UncheckedAutoCloseable measure(KEY key) {
return () -> {
concurrency.decrementAndGet();
counters.get(key).incrementAndGet();
elapsedTimes.get(key).addAndGet(System.currentTimeMillis() - startTime);
long delta = System.currentTimeMillis() - startTime;
elapsedTimes.get(key).addAndGet(delta);
if (quantileEnable) {
for (MutableQuantiles q : quantiles.get(key)) {
q.add(delta);
}
}
};
}

Expand All @@ -89,6 +126,7 @@ public void unregister() {

@Override
public void getMetrics(MetricsCollector collector, boolean all) {
registry.snapshot(collector.addRecord(registry.info()), all);
Comment thread
xBis7 marked this conversation as resolved.
Outdated
counters.forEach((key, value) -> {
MetricsRecordBuilder builder =
collector.addRecord(name);
Expand Down Expand Up @@ -129,4 +167,8 @@ public String description() {
return description;
}
}

public boolean isQuantileEnable() {
Comment thread
xBis7 marked this conversation as resolved.
Outdated
return quantileEnable;
}
}
3 changes: 2 additions & 1 deletion ...ds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ public SCMBlockProtocolServer(OzoneConfiguration conf,
ProtocolMessageMetrics.create(
"ScmBlockLocationProtocol",
"SCM Block location protocol counters",
ScmBlockLocationProtocolProtos.Type.values());
ScmBlockLocationProtocolProtos.Type.values(),
conf);

// SCM Block Service RPC.
BlockingService blockProtoPbService =
Expand Down
3 changes: 2 additions & 1 deletion ...s/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ public SCMClientProtocolServer(OzoneConfiguration conf,
protocolMetrics = ProtocolMessageMetrics
.create("ScmContainerLocationProtocol",
"SCM ContainerLocation protocol metrics",
StorageContainerLocationProtocolProtos.Type.values());
StorageContainerLocationProtocolProtos.Type.values(),
conf);

// SCM Container Service RPC
BlockingService storageProtoPbService =
Expand Down
7 changes: 4 additions & 3 deletions ...server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMDatanodeProtocolServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.util.concurrent.TimeoutException;

import org.apache.hadoop.fs.CommonConfigurationKeys;
import org.apache.hadoop.hdds.conf.ConfigurationSource;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.protocol.DatanodeDetails;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
Expand Down Expand Up @@ -156,7 +157,7 @@ public SCMDatanodeProtocolServer(final OzoneConfiguration conf,
InetSocketAddress datanodeRpcAddr = getDataNodeBindAddress(
conf, scm.getScmNodeDetails());

protocolMessageMetrics = getProtocolMessageMetrics();
protocolMessageMetrics = getProtocolMessageMetrics(conf);

final int handlerCount = conf.getInt(OZONE_SCM_HANDLER_COUNT_KEY,
OZONE_SCM_HANDLER_COUNT_DEFAULT);
Expand Down Expand Up @@ -469,10 +470,10 @@ private static String flatten(String input) {
* @return ProtocolMessageMetrics
*/
protected ProtocolMessageMetrics<ProtocolMessageEnum>
getProtocolMessageMetrics() {
getProtocolMessageMetrics(ConfigurationSource conf) {
return ProtocolMessageMetrics
.create("SCMDatanodeProtocol", "SCM Datanode protocol",
StorageContainerDatanodeProtocolProtos.Type.values());
StorageContainerDatanodeProtocolProtos.Type.values(), conf);
}

/**
Expand Down
10 changes: 6 additions & 4 deletions ...server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMSecurityProtocolServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,12 +145,14 @@ public class SCMSecurityProtocolServer implements SCMSecurityProtocol,
// SCM security service RPC service.
RPC.setProtocolEngine(conf, SCMSecurityProtocolPB.class,
ProtobufRpcEngine.class);
metrics = new ProtocolMessageMetrics("ScmSecurityProtocol",
metrics = ProtocolMessageMetrics.create("ScmSecurityProtocol",
"SCM Security protocol metrics",
SCMSecurityProtocolProtos.Type.values());
secretKeyMetrics = new ProtocolMessageMetrics("ScmSecretKeyProtocol",
SCMSecurityProtocolProtos.Type.values(),
conf);
secretKeyMetrics = ProtocolMessageMetrics.create("ScmSecretKeyProtocol",
"SCM SecretKey protocol metrics",
SCMSecretKeyProtocolProtos.Type.values());
SCMSecretKeyProtocolProtos.Type.values(),
conf);
BlockingService secureProtoPbService =
SCMSecurityProtocolProtos.SCMSecurityProtocolService
.newReflectiveBlockingService(
Expand Down
1 change: 1 addition & 0 deletions .../integration-test/src/test/java/org/apache/hadoop/ozone/TestOzoneConfigurationFields.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ private void addPropertiesNotInXml() {
OzoneConfigKeys.OZONE_RECOVERING_CONTAINER_SCRUBBING_SERVICE_TIMEOUT,
OzoneConfigKeys.OZONE_RECOVERING_CONTAINER_TIMEOUT,
OzoneConfigKeys.OZONE_GPRC_METRICS_PERCENTILES_INTERVALS_KEY,
OzoneConfigKeys.OZONE_PROTOCOL_MESSAGE_METRICS_PERCENTILES_INTERVALS,
ReconConfigKeys.RECON_SCM_CONFIG_PREFIX,
ReconConfigKeys.OZONE_RECON_ADDRESS_KEY,
ReconConfigKeys.OZONE_RECON_DATANODE_ADDRESS_KEY,
Expand Down
2 changes: 1 addition & 1 deletion hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,7 @@ private OzoneManager(OzoneConfiguration conf, StartupOption startupOption)

omClientProtocolMetrics = ProtocolMessageMetrics
.create("OmClientProtocol", "Ozone Manager RPC endpoint",
OzoneManagerProtocolProtos.Type.values());
OzoneManagerProtocolProtos.Type.values(), conf);

// Start Om Rpc Server.
omRpcServer = getRpcServer(configuration);
Expand Down
5 changes: 3 additions & 2 deletions ...ne/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconDatanodeProtocolServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.io.IOException;
import java.net.InetSocketAddress;

import org.apache.hadoop.hdds.conf.ConfigurationSource;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos;
import org.apache.hadoop.hdds.scm.ha.SCMNodeDetails;
Expand Down Expand Up @@ -51,10 +52,10 @@ public ReconDatanodeProtocolServer(OzoneConfiguration conf,

@Override
public ProtocolMessageMetrics<ProtocolMessageEnum>
getProtocolMessageMetrics() {
getProtocolMessageMetrics(ConfigurationSource conf) {
return ProtocolMessageMetrics
.create("ReconDatanodeProtocol", "Recon Datanode protocol",
StorageContainerDatanodeProtocolProtos.Type.values());
StorageContainerDatanodeProtocolProtos.Type.values(), conf);
}

@Override
Expand Down