errose28 commented on code in PR #7206:
URL: https://github.com/apache/ozone/pull/7206#discussion_r1763592439
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java:
##########
@@ -303,15 +304,17 @@ public void start() {
datanodeDetails.setPort(DatanodeDetails.newPort(HTTPS,
httpServer.getHttpsAddress().getPort()));
}
+
serviceRuntimeInfo.setHttpPort(String.valueOf(httpServer.getHttpAddress().getPort()));
Review Comment:
What about the HTTPS port and address? are other components publishing these
as separate metrics?
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/BackgroundContainerDataScanner.java:
##########
@@ -110,9 +112,19 @@ public void scanContainer(Container<?> c)
@Override
public Iterator<Container<?>> getContainerIterator() {
+ recordContainersMetric();
Review Comment:
This is only called at the beginning of a background scan, which on dense
volumes may happen once every few weeks. Real time updates as containers are
moved, created, deleted, should be plugged directly into the `ContainerSet`.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeInfoMetrics.java:
##########
@@ -153,4 +154,15 @@ public void dbCompactTimesNanoSecondsIncr(long time) {
dbCompactLatency.add(time);
}
+ /**
+ * Return the Container Count of the Volume.
+ */
+ @Metric("Returns the Container Count of the Volume")
+ public long getContainers() {
+ return this.containers;
+ }
+
+ public void setContainers(long count) {
+ this.containers = count;
+ }
Review Comment:
This class currently pulls most of its metrics from the `volume` field and
does not depend on external setters except for `dbCompactLatency`. Can we do
the same for this metric?
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ContainerSet.java:
##########
@@ -252,6 +252,21 @@ public Iterator<Container<?>>
getContainerIterator(HddsVolume volume) {
.iterator();
}
+ /**
+ * Get the number of containers based on the given volume.
+ *
+ * @param volume hdds volume.
+ * @return number of containers
+ */
+ public long getContainerCount(HddsVolume volume) {
Review Comment:
```suggestion
public long containerCount(HddsVolume volume) {
```
Then we can change the existing `containerCount` method to return a long,
making the two variants overloads.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java:
##########
@@ -303,15 +304,17 @@ public void start() {
datanodeDetails.setPort(DatanodeDetails.newPort(HTTPS,
httpServer.getHttpsAddress().getPort()));
}
+
serviceRuntimeInfo.setHttpPort(String.valueOf(httpServer.getHttpAddress().getPort()));
} catch (Exception ex) {
LOG.error("HttpServer failed to start.", ex);
}
-
clientProtocolServer = new HddsDatanodeClientProtocolServer(
datanodeDetails, conf, HddsVersionInfo.HDDS_VERSION_INFO,
reconfigurationHandler);
+
serviceRuntimeInfo.setRpcPort(String.valueOf(clientProtocolServer.getClientRpcAddress().getPort()));
Review Comment:
The datanode has multiple RPC ports for various purposes, see
[here](https://github.com/apache/ozone/blob/624bede619004a6a2be5fe185abd40c2999ae3c1/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java#L896).
This one is specifically the one used by the client for config reload. Do
other components have an RPC port metric, and do they add more information
about what this port is used for?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
