You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/06/09 17:44:31 UTC
[GitHub] [ozone] yuangu002 opened a new pull request #2321: HDDS-5314. Show number of Open vs Closed containers per Node in Recon UI
yuangu002 opened a new pull request #2321:
URL: https://github.com/apache/ozone/pull/2321
## What changes were proposed in this pull request?
The Recon UI DataNode page only shows the total number of containers. This jira aims to show the open / closed containers count on the UI.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5314
## How was this patch tested?
Tested it on UI, and use "ozone admin container close" to close a container.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] yuangu002 commented on a change in pull request #2321: HDDS-5314. Show number of Open containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
yuangu002 commented on a change in pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#discussion_r652164122
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NodeEndpoint.java
##########
@@ -117,11 +123,26 @@ public Response getDatanodes() {
}
});
try {
- int containers = nodeManager.getContainers(datanode).size();
+ Set<ContainerID> allContainers = nodeManager.getContainers(datanode);
+
+ int openContainers = 0;
+ for (ContainerID containerID: allContainers) {
+ ContainerInfo containerInfo = reconContainerManager.getContainer(containerID);
Review comment:
fixed, please review.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl merged pull request #2321: HDDS-5314. Show number of Open containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
smengcl merged pull request #2321:
URL: https://github.com/apache/ozone/pull/2321
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] yuangu002 edited a comment on pull request #2321: HDDS-5314. Show number of Open vs Closed containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
yuangu002 edited a comment on pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#issuecomment-858988306
Fixed:
1. style
2. update db.json with dummy data
Test:
```
ozone sh volume create --namespace-quota=5 --quota=5GB /vol
ozone sh bucket create --namespace-quota=5 --quota=5GB /vol/bucket
ozone sh key put /vol/bucket/README.md README.md
ozone sh key put -r=THREE /vol/bucket/CONTRIBUTING.md CONTRIBUTING.md
```
All containers are open:
Then close container 1: `ozone admin container close 1`, one container is closed
Then close container 2: `ozone admin container close 2`, all containers are closed
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] yuangu002 commented on pull request #2321: HDDS-5314. Show number of Open vs Closed containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
yuangu002 commented on pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#issuecomment-858988306
Fixed:
1. style
2. update db.json with dummy data
Test:
```
ozone sh volume create --namespace-quota=5 --quota=5GB /vol
ozone sh bucket create --namespace-quota=5 --quota=5GB /vol/bucket
ozone sh key put /vol/bucket/README.md README.md
ozone sh key put -r=THREE /vol/bucket/CONTRIBUTING.md CONTRIBUTING.md
```
All containers are open:
Then close container 1: `ozone admin container close 1`, one container is closed
Then close container 1: `ozone admin container close 2`, all containers are closed
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] yuangu002 commented on a change in pull request #2321: HDDS-5314. Show number of Open vs Closed containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
yuangu002 commented on a change in pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#discussion_r649487744
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NodeEndpoint.java
##########
@@ -117,11 +123,26 @@ public Response getDatanodes() {
}
});
try {
- int containers = nodeManager.getContainers(datanode).size();
+ Set<ContainerID> allContainers = nodeManager.getContainers(datanode);
+
+ int openContainers = 0;
+ for (ContainerID containerID: allContainers) {
+ ContainerInfo containerInfo = reconContainerManager.getContainer(containerID);
+ if (containerInfo.isOpen()) {
+ ++openContainers;
+ }
+ }
+
+ int containers = allContainers.size();
+ int closedContainers = containers - openContainers;
builder.withContainers(containers);
+ builder.withOpenContainers(openContainers);
+ builder.withClosedContainers(closedContainers);
} catch (NodeNotFoundException ex) {
LOG.warn("Cannot get containers, datanode {} not found.",
datanode.getUuid(), ex);
+ } catch (ContainerNotFoundException cnfe) {
+ LOG.warn("Cannot found container.", cnfe);
Review comment:
Fixed.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2321: HDDS-5314. Show number of Open vs Closed containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#discussion_r649488357
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NodeEndpoint.java
##########
@@ -117,11 +123,26 @@ public Response getDatanodes() {
}
});
try {
- int containers = nodeManager.getContainers(datanode).size();
+ Set<ContainerID> allContainers = nodeManager.getContainers(datanode);
+
+ int openContainers = 0;
+ for (ContainerID containerID: allContainers) {
+ ContainerInfo containerInfo = reconContainerManager.getContainer(containerID);
+ if (containerInfo.isOpen()) {
+ ++openContainers;
+ }
+ }
Review comment:
Could this loop be eliminated? There might be an existing count lying somewhere.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] avijayanhwx commented on a change in pull request #2321: HDDS-5314. Show number of Open vs Closed containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#discussion_r649454623
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/DatanodeMetadata.java
##########
@@ -56,6 +56,12 @@
@XmlElement(name = "containers")
private int containers;
+ @XmlElement(name = "openContainers")
+ private int openContainers;
+
+ @XmlElement(name = "closedContainers")
Review comment:
Since we have other transient states from OPEN to CLOSED, maybe we can just track openContainers & totalContainers. It is the 'OPEN' container count per node which is a significant data point for writes.
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NodeEndpoint.java
##########
@@ -117,11 +123,26 @@ public Response getDatanodes() {
}
});
try {
- int containers = nodeManager.getContainers(datanode).size();
+ Set<ContainerID> allContainers = nodeManager.getContainers(datanode);
+
+ int openContainers = 0;
+ for (ContainerID containerID: allContainers) {
+ ContainerInfo containerInfo = reconContainerManager.getContainer(containerID);
Review comment:
When a container goes from OPEN to CLOSED it is meant to be an irreversible change, and it stays CLOSED (or DELETED). Over the lifetime of the cluster, we expect the number of OPEN containers (which are newly created) to remain stable, while the CLOSED containers will keep increasing. If we iterate over all the containers in the system for every call, then most of the computation will remain unchanged (since CLOSED containers will remain that way). Given the above, can we think of a way to optimize this using cached information?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2321: HDDS-5314. Show number of Open containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#discussion_r652171758
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconContainerManager.java
##########
@@ -203,6 +207,15 @@ private void checkContainerStateAndUpdate(ContainerID containerID,
&& isHealthy(state)) {
LOG.info("Container {} has state OPEN, but given state is {}.",
containerID, state);
+ Pipeline pipeline =
+ pipelineManager.getPipeline(containerInfo.getPipelineID());
+ // subtract open container count from the pipeline
+ int curCnt = pipelineToOpenContainer.getOrDefault(pipeline.getId(), 0);
+ if (curCnt == 1) {
+ pipelineToOpenContainer.remove(pipeline.getId());
+ } else if (curCnt > 0) {
+ pipelineToOpenContainer.put(pipeline.getId(), curCnt - 1);
+ }
Review comment:
Can we use `containerInfo.getPipelineID()` directly?
Seems we are converting from `PipelineID` to `Pipeline` back to the same `PipelineID`?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2321: HDDS-5314. Show number of Open vs Closed containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#discussion_r652101619
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/CloseContainerEventHandler.java
##########
@@ -85,9 +85,11 @@ public void onMessage(ContainerID containerID, EventPublisher publisher) {
command.setTerm(scmContext.getTermOfLeader());
command.setEncodedToken(getContainerToken(containerID));
- getNodes(container).forEach(node ->
- publisher.fireEvent(DATANODE_COMMAND,
- new CommandForDatanode<>(node.getUuid(), command)));
+ getNodes(container).forEach(node -> {
+ publisher.fireEvent(DATANODE_COMMAND,
+ new CommandForDatanode<>(node.getUuid(), command));
+ });
+
Review comment:
Let's remove unnecessary changes in this file.
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NodeEndpoint.java
##########
@@ -108,6 +115,10 @@ public Response getDatanodes() {
if (datanode.getUuid().equals(pipeline.getLeaderId())) {
leaderCount.getAndIncrement();
}
+ int openContainerPerPipeline =
+ reconContainerManager.getPipelineToOpenContainer()
+ .getOrDefault(pipelineID, 0);
+ openContainers.set(openContainers.get() + openContainerPerPipeline);
Review comment:
Doesn't seem atomic to me.. Try this?
```suggestion
openContainers.getAndAdd(openContainerPerPipeline);
```
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NodeEndpoint.java
##########
@@ -108,6 +115,10 @@ public Response getDatanodes() {
if (datanode.getUuid().equals(pipeline.getLeaderId())) {
leaderCount.getAndIncrement();
}
+ int openContainerPerPipeline =
+ reconContainerManager.getPipelineToOpenContainer()
+ .getOrDefault(pipelineID, 0);
Review comment:
What is the rationale behind defaulting to pipeline ID `0` here? Wouldn't `get(pipelineID)` be better?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2321: HDDS-5314. Show number of Open vs Closed containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#discussion_r652108474
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NodeEndpoint.java
##########
@@ -108,6 +115,10 @@ public Response getDatanodes() {
if (datanode.getUuid().equals(pipeline.getLeaderId())) {
leaderCount.getAndIncrement();
}
+ int openContainerPerPipeline =
+ reconContainerManager.getPipelineToOpenContainer()
+ .getOrDefault(pipelineID, 0);
Review comment:
~~What is the rationale behind defaulting to pipeline ID `0` here? Wouldn't `get(pipelineID)` be better?~~ nvm I understood it wrong
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2321: HDDS-5314. Show number of Open vs Closed containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#discussion_r652154033
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconContainerManager.java
##########
@@ -70,6 +71,8 @@
private final Table<UUID, DatanodeDetails> nodeDB;
// Container ID -> Datanode UUID -> Timestamp
private final Map<Long, Map<UUID, ContainerReplicaHistory>> replicaHistoryMap;
+ // Pipeline -> open containers
+ private Map<PipelineID, Integer> pipelineToOpenContainer;
Review comment:
```suggestion
private final Map<PipelineID, Integer> pipelineToOpenContainer;
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2321: HDDS-5314. Show number of Open vs Closed containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#discussion_r651268001
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/NodeStateMap.java
##########
@@ -317,6 +317,8 @@ public void addContainer(final UUID uuid,
lock.writeLock().lock();
try {
checkIfNodeExist(uuid);
+ // SCMContainerManager.getContainer() -> containerInfo
+
Review comment:
```suggestion
```
also accidental?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] vivekratnavel commented on a change in pull request #2321: HDDS-5314. Show number of Open vs Closed containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
vivekratnavel commented on a change in pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#discussion_r649418700
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/NodeEndpoint.java
##########
@@ -117,11 +123,26 @@ public Response getDatanodes() {
}
});
try {
- int containers = nodeManager.getContainers(datanode).size();
+ Set<ContainerID> allContainers = nodeManager.getContainers(datanode);
+
+ int openContainers = 0;
+ for (ContainerID containerID: allContainers) {
+ ContainerInfo containerInfo = reconContainerManager.getContainer(containerID);
+ if (containerInfo.isOpen()) {
+ ++openContainers;
+ }
+ }
+
+ int containers = allContainers.size();
+ int closedContainers = containers - openContainers;
builder.withContainers(containers);
+ builder.withOpenContainers(openContainers);
+ builder.withClosedContainers(closedContainers);
} catch (NodeNotFoundException ex) {
LOG.warn("Cannot get containers, datanode {} not found.",
datanode.getUuid(), ex);
+ } catch (ContainerNotFoundException cnfe) {
+ LOG.warn("Cannot found container.", cnfe);
Review comment:
nit:
```suggestion
LOG.warn("Cannot find container.", cnfe);
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2321: HDDS-5314. Show number of Open vs Closed containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#discussion_r652142346
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconContainerManager.java
##########
@@ -70,6 +71,8 @@
private final Table<UUID, DatanodeDetails> nodeDB;
// Container ID -> Datanode UUID -> Timestamp
private final Map<Long, Map<UUID, ContainerReplicaHistory>> replicaHistoryMap;
+ // Pipeline -> open containers
+ private Map<PipelineID, Integer> pipelineToOpenContainer;
Review comment:
```suggestion
private Map<PipelineID, AtomicInteger> pipelineToOpenContainer;
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2321: HDDS-5314. Show number of Open containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#discussion_r652171758
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconContainerManager.java
##########
@@ -203,6 +207,15 @@ private void checkContainerStateAndUpdate(ContainerID containerID,
&& isHealthy(state)) {
LOG.info("Container {} has state OPEN, but given state is {}.",
containerID, state);
+ Pipeline pipeline =
+ pipelineManager.getPipeline(containerInfo.getPipelineID());
+ // subtract open container count from the pipeline
+ int curCnt = pipelineToOpenContainer.getOrDefault(pipeline.getId(), 0);
+ if (curCnt == 1) {
+ pipelineToOpenContainer.remove(pipeline.getId());
+ } else if (curCnt > 0) {
+ pipelineToOpenContainer.put(pipeline.getId(), curCnt - 1);
+ }
Review comment:
Can we use `containerInfo.getPipelineID()` directly?
Seems we are converting from `PipelineID` to `Pipeline` then back to the same `PipelineID`?
```suggestion
final PipelineID pipelineID = containerInfo.getPipelineID();
// subtract open container count from the map
int curCnt = pipelineToOpenContainer.getOrDefault(pipelineID, 0);
if (curCnt == 1) {
pipelineToOpenContainer.remove(pipelineID);
} else if (curCnt > 0) {
pipelineToOpenContainer.put(pipelineID, curCnt - 1);
}
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2321: HDDS-5314. Show number of Open vs Closed containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#discussion_r651262821
##########
File path: hadoop-hdds/common/audit.log
##########
@@ -0,0 +1 @@
+ERROR | OMAudit | ? | user=john | ip=192.168.0.1 | op=CREATE_VOLUME {key1=value1, key2=value2} | ret=FAILURE
Review comment:
pushed by accident?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on pull request #2321: HDDS-5314. Show number of Open containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#issuecomment-866189506
Thanks @yuangu002 for the contribution. And thanks @avijayanhwx @vivekratnavel for the review.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] vivekratnavel commented on pull request #2321: HDDS-5314. Show number of Open vs Closed containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
vivekratnavel commented on pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#issuecomment-858871004
Also, please fix the checkstyle issues reported [here](https://github.com/apache/ozone/pull/2321/checks?check_run_id=2786593331)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] yuangu002 commented on a change in pull request #2321: HDDS-5314. Show number of Open vs Closed containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
yuangu002 commented on a change in pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#discussion_r651274843
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/NodeStateMap.java
##########
@@ -317,6 +317,8 @@ public void addContainer(final UUID uuid,
lock.writeLock().lock();
try {
checkIfNodeExist(uuid);
+ // SCMContainerManager.getContainer() -> containerInfo
+
Review comment:
Yeah, demo lines. Fixed
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2321: HDDS-5314. Show number of Open containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#discussion_r652171758
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconContainerManager.java
##########
@@ -203,6 +207,15 @@ private void checkContainerStateAndUpdate(ContainerID containerID,
&& isHealthy(state)) {
LOG.info("Container {} has state OPEN, but given state is {}.",
containerID, state);
+ Pipeline pipeline =
+ pipelineManager.getPipeline(containerInfo.getPipelineID());
+ // subtract open container count from the pipeline
+ int curCnt = pipelineToOpenContainer.getOrDefault(pipeline.getId(), 0);
+ if (curCnt == 1) {
+ pipelineToOpenContainer.remove(pipeline.getId());
+ } else if (curCnt > 0) {
+ pipelineToOpenContainer.put(pipeline.getId(), curCnt - 1);
+ }
Review comment:
Can we use `containerInfo.getPipelineID()` directly?
Seems we are converting from `PipelineID` to `Pipeline` then back to the same `PipelineID`?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2321: HDDS-5314. Show number of Open containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#discussion_r652169666
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconContainerManager.java
##########
@@ -70,6 +71,8 @@
private final Table<UUID, DatanodeDetails> nodeDB;
// Container ID -> Datanode UUID -> Timestamp
private final Map<Long, Map<UUID, ContainerReplicaHistory>> replicaHistoryMap;
+ // Pipeline -> open containers
Review comment:
nit
```suggestion
// Pipeline -> # of open containers
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2321: HDDS-5314. Show number of Open vs Closed containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#discussion_r652122145
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconContainerManager.java
##########
@@ -203,6 +207,13 @@ private void checkContainerStateAndUpdate(ContainerID containerID,
&& isHealthy(state)) {
LOG.info("Container {} has state OPEN, but given state is {}.",
containerID, state);
+ Pipeline pipeline =
+ pipelineManager.getPipeline(containerInfo.getPipelineID());
+ // subtract open container count from the pipeline
+ int curCnt = pipelineToOpenContainer.getOrDefault(pipeline.getId(), 0);
+ if (curCnt > 0) {
+ pipelineToOpenContainer.put(pipeline.getId(), curCnt - 1);
+ }
Review comment:
```suggestion
// We assume that this method is single-threaded
int curCnt = pipelineToOpenContainer.getOrDefault(pipeline.getId(), 0);
if (curCnt == 1) {
pipelineToOpenContainer.remove(pipeline.getId());
} else if (curCnt > 0) {
pipelineToOpenContainer.put(pipeline.getId(), curCnt - 1);
}
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2321: HDDS-5314. Show number of Open vs Closed containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#discussion_r652122145
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconContainerManager.java
##########
@@ -203,6 +207,13 @@ private void checkContainerStateAndUpdate(ContainerID containerID,
&& isHealthy(state)) {
LOG.info("Container {} has state OPEN, but given state is {}.",
containerID, state);
+ Pipeline pipeline =
+ pipelineManager.getPipeline(containerInfo.getPipelineID());
+ // subtract open container count from the pipeline
+ int curCnt = pipelineToOpenContainer.getOrDefault(pipeline.getId(), 0);
+ if (curCnt > 0) {
+ pipelineToOpenContainer.put(pipeline.getId(), curCnt - 1);
+ }
Review comment:
```suggestion
int curCnt = pipelineToOpenContainer.getOrDefault(pipeline.getId(), 0);
if (curCnt == 1) {
pipelineToOpenContainer.remove(pipeline.getId());
} else if (curCnt > 0) {
pipelineToOpenContainer.put(pipeline.getId(), curCnt - 1);
}
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2321: HDDS-5314. Show number of Open vs Closed containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#discussion_r652124228
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconContainerManager.java
##########
@@ -223,12 +234,16 @@ public void addNewContainer(ContainerWithPipeline containerWithPipeline)
ContainerInfo containerInfo = containerWithPipeline.getContainerInfo();
try {
if (containerInfo.getState().equals(HddsProtos.LifeCycleState.OPEN)) {
- PipelineID pipelineID = containerWithPipeline.getPipeline().getId();
+ Pipeline pipeline = containerWithPipeline.getPipeline();
+ PipelineID pipelineID = pipeline.getId();
if (pipelineManager.containsPipeline(pipelineID)) {
getContainerStateManager().addContainer(containerInfo.getProtobuf());
pipelineManager.addContainerToPipeline(
containerWithPipeline.getPipeline().getId(),
containerInfo.containerID());
+ // update open container count on all datanodes on this pipeline
+ pipelineToOpenContainer.put(pipelineID,
+ pipelineToOpenContainer.getOrDefault(pipelineID, 0) + 1);
Review comment:
Use AtomicInteger for the value
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2321: HDDS-5314. Show number of Open containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#discussion_r652174327
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconContainerManager.java
##########
@@ -223,12 +236,16 @@ public void addNewContainer(ContainerWithPipeline containerWithPipeline)
ContainerInfo containerInfo = containerWithPipeline.getContainerInfo();
try {
if (containerInfo.getState().equals(HddsProtos.LifeCycleState.OPEN)) {
- PipelineID pipelineID = containerWithPipeline.getPipeline().getId();
+ Pipeline pipeline = containerWithPipeline.getPipeline();
+ PipelineID pipelineID = pipeline.getId();
Review comment:
Let's also restore these two lines. The change doesn't seem necessary any more.
```suggestion
PipelineID pipelineID = containerWithPipeline.getPipeline().getId();
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2321: HDDS-5314. Show number of Open containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#discussion_r652174327
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconContainerManager.java
##########
@@ -223,12 +236,16 @@ public void addNewContainer(ContainerWithPipeline containerWithPipeline)
ContainerInfo containerInfo = containerWithPipeline.getContainerInfo();
try {
if (containerInfo.getState().equals(HddsProtos.LifeCycleState.OPEN)) {
- PipelineID pipelineID = containerWithPipeline.getPipeline().getId();
+ Pipeline pipeline = containerWithPipeline.getPipeline();
+ PipelineID pipelineID = pipeline.getId();
Review comment:
Let's also restore these two lines, doesn't seem necessary any more.
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconContainerManager.java
##########
@@ -223,12 +236,16 @@ public void addNewContainer(ContainerWithPipeline containerWithPipeline)
ContainerInfo containerInfo = containerWithPipeline.getContainerInfo();
try {
if (containerInfo.getState().equals(HddsProtos.LifeCycleState.OPEN)) {
- PipelineID pipelineID = containerWithPipeline.getPipeline().getId();
+ Pipeline pipeline = containerWithPipeline.getPipeline();
+ PipelineID pipelineID = pipeline.getId();
Review comment:
Let's also restore these two lines. The change doesn't seem necessary any more.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a change in pull request #2321: HDDS-5314. Show number of Open vs Closed containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#discussion_r652142346
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconContainerManager.java
##########
@@ -70,6 +71,8 @@
private final Table<UUID, DatanodeDetails> nodeDB;
// Container ID -> Datanode UUID -> Timestamp
private final Map<Long, Map<UUID, ContainerReplicaHistory>> replicaHistoryMap;
+ // Pipeline -> open containers
+ private Map<PipelineID, Integer> pipelineToOpenContainer;
Review comment:
```suggestion
private Map<PipelineID, AtomicInteger> pipelineToOpenContainer;
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] yuangu002 commented on a change in pull request #2321: HDDS-5314. Show number of Open vs Closed containers per Node in Recon UI
Posted by GitBox <gi...@apache.org>.
yuangu002 commented on a change in pull request #2321:
URL: https://github.com/apache/ozone/pull/2321#discussion_r649486358
##########
File path: hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/DatanodeMetadata.java
##########
@@ -56,6 +56,12 @@
@XmlElement(name = "containers")
private int containers;
+ @XmlElement(name = "openContainers")
+ private int openContainers;
+
+ @XmlElement(name = "closedContainers")
Review comment:
Got it, will fix that soon
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org