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 2019/12/24 02:55:59 UTC
[GitHub] [hadoop-ozone] chimney-lee opened a new pull request #389:
HDDS-2534. scmcli container delete not working
chimney-lee opened a new pull request #389: HDDS-2534. scmcli container delete not working
URL: https://github.com/apache/hadoop-ozone/pull/389
## What changes were proposed in this pull request?
What changes were proposed in this pull request?
fix scmcli container delete not working
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-2534
#214
How was this patch tested?
exec bin/ozone scmcli container delete <containerId>
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] avijayanhwx commented on a change in pull request
#389: HDDS-2534. scmcli container delete not working
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #389: HDDS-2534. scmcli container delete not working
URL: https://github.com/apache/hadoop-ozone/pull/389#discussion_r367111654
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerReportHandler.java
##########
@@ -119,21 +127,33 @@ public void onMessage(final ContainerReportFromDatanode reportFromDatanode,
* @param datanodeDetails Datanode from which this report was received
* @param replicas list of ContainerReplicaProto
*/
- private void processContainerReplicas(final DatanodeDetails datanodeDetails,
- final List<ContainerReplicaProto> replicas) {
+ private List<ContainerReplicaProto> processContainerReplicas(
+ final DatanodeDetails datanodeDetails,
+ final List<ContainerReplicaProto> replicas,
+ final EventPublisher publisher) {
+ List<ContainerReplicaProto> containers = new ArrayList<>();
for (ContainerReplicaProto replicaProto : replicas) {
try {
processContainerReplica(datanodeDetails, replicaProto);
+ containers.add(replicaProto);
} catch (ContainerNotFoundException e) {
- LOG.error("Received container report for an unknown container" +
- " {} from datanode {}.", replicaProto.getContainerID(),
- datanodeDetails, e);
+ LOG.info("Received container report for an unknown container" +
+ " {} from datanode {},delete it.",
+ replicaProto.getContainerID(),
+ datanodeDetails);
+ publisher.fireEvent(
Review comment:
+1 for the description, the delete container flow is quite important for Recon to work correctly and hence trying to understand this better. Will there be a case where a user will need to delete a container directly using SCM?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] elek commented on issue #389: HDDS-2534. scmcli
container delete not working
Posted by GitBox <gi...@apache.org>.
elek commented on issue #389: HDDS-2534. scmcli container delete not working
URL: https://github.com/apache/hadoop-ozone/pull/389#issuecomment-597084180
/pending Questions/suggestions from @xiaoyuyao are not yet addressed in #432 12f3f8ac94cf8808757bb7673e4208d8b0fede09
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] elek removed a comment on issue #389: HDDS-2534.
scmcli container delete not working
Posted by GitBox <gi...@apache.org>.
elek removed a comment on issue #389: HDDS-2534. scmcli container delete not working
URL: https://github.com/apache/hadoop-ozone/pull/389#issuecomment-615093783
/close
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] elek commented on issue #389: HDDS-2534. scmcli
container delete not working
Posted by GitBox <gi...@apache.org>.
elek commented on issue #389: HDDS-2534. scmcli container delete not working
URL: https://github.com/apache/hadoop-ozone/pull/389#issuecomment-615093783
/close
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] chimney-lee commented on issue #389: HDDS-2534.
scmcli container delete not working
Posted by GitBox <gi...@apache.org>.
chimney-lee commented on issue #389: HDDS-2534. scmcli container delete not working
URL: https://github.com/apache/hadoop-ozone/pull/389#issuecomment-568820374
@xiaoyuyao please review. relates to #214
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#389: HDDS-2534. scmcli container delete not working
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #389: HDDS-2534. scmcli container delete not working
URL: https://github.com/apache/hadoop-ozone/pull/389#discussion_r366639245
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
##########
@@ -563,6 +563,20 @@ public void setContainers(DatanodeDetails datanodeDetails,
return nodeStateManager.getContainers(datanodeDetails.getUuid());
}
+ /**
+ * Remove set of containers available on a datanode.
+ *
+ * @param datanodeDetails - DatanodeID
+ * @param containerIds - Set of containerIDs
+ * @throws NodeNotFoundException - if datanode is not known. For new datanode
+ * use addDatanodeInContainerMap call.
+ */
+ @Override
+ public void removeContainers(DatanodeDetails datanodeDetails,
+ Set<ContainerID> containerIds) throws NodeNotFoundException {
+ nodeStateManager.removeContainers(datanodeDetails.getUuid(), containerIds);
Review comment:
Do we need to remove the container from the pipelineContainerMap in SCMPipelineManager?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#389: HDDS-2534. scmcli container delete not working
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #389: HDDS-2534. scmcli container delete not working
URL: https://github.com/apache/hadoop-ozone/pull/389#discussion_r366605305
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/SCMContainerManager.java
##########
@@ -282,6 +290,12 @@ public ContainerInfo allocateContainer(final ReplicationType type,
public void deleteContainer(ContainerID containerID) throws IOException {
lock.lock();
try {
+ Set<ContainerReplica> replicas =
+ containerStateManager.getContainerReplicas(containerID);
+ for(ContainerReplica replica : replicas){
+ nodeManager.removeContainers(replica.getDatanodeDetails(), Stream.of(
Review comment:
Can we create the set of containerID outside the loop with a simpler Java APIs:
Set<ContainerID> containerIdSet = Collections.singleton(containerID);
replias.forEach(replica->nodeManager.removeContainers(replica.getDatanodeDetails(), containerIdSet));
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#389: HDDS-2534. scmcli container delete not working
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #389: HDDS-2534. scmcli container delete not working
URL: https://github.com/apache/hadoop-ozone/pull/389#discussion_r366638690
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerReportHandler.java
##########
@@ -119,21 +127,33 @@ public void onMessage(final ContainerReportFromDatanode reportFromDatanode,
* @param datanodeDetails Datanode from which this report was received
* @param replicas list of ContainerReplicaProto
*/
- private void processContainerReplicas(final DatanodeDetails datanodeDetails,
- final List<ContainerReplicaProto> replicas) {
+ private List<ContainerReplicaProto> processContainerReplicas(
+ final DatanodeDetails datanodeDetails,
+ final List<ContainerReplicaProto> replicas,
+ final EventPublisher publisher) {
+ List<ContainerReplicaProto> containers = new ArrayList<>();
for (ContainerReplicaProto replicaProto : replicas) {
try {
processContainerReplica(datanodeDetails, replicaProto);
+ containers.add(replicaProto);
} catch (ContainerNotFoundException e) {
- LOG.error("Received container report for an unknown container" +
- " {} from datanode {}.", replicaProto.getContainerID(),
- datanodeDetails, e);
+ LOG.info("Received container report for an unknown container" +
+ " {} from datanode {},delete it.",
+ replicaProto.getContainerID(),
+ datanodeDetails);
+ publisher.fireEvent(
Review comment:
Should we fireEvent upon receive the scmcli request? Can we add some high level design description wrt the flow of the delete in the PR description? That will help the reviewers.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] chimney-lee commented on a change in pull request
#389: HDDS-2534. scmcli container delete not working
Posted by GitBox <gi...@apache.org>.
chimney-lee commented on a change in pull request #389: HDDS-2534. scmcli container delete not working
URL: https://github.com/apache/hadoop-ozone/pull/389#discussion_r368273738
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
##########
@@ -563,6 +563,20 @@ public void setContainers(DatanodeDetails datanodeDetails,
return nodeStateManager.getContainers(datanodeDetails.getUuid());
}
+ /**
+ * Remove set of containers available on a datanode.
+ *
+ * @param datanodeDetails - DatanodeID
+ * @param containerIds - Set of containerIDs
+ * @throws NodeNotFoundException - if datanode is not known. For new datanode
+ * use addDatanodeInContainerMap call.
+ */
+ @Override
+ public void removeContainers(DatanodeDetails datanodeDetails,
+ Set<ContainerID> containerIds) throws NodeNotFoundException {
+ nodeStateManager.removeContainers(datanodeDetails.getUuid(), containerIds);
Review comment:
Before delete a container, we should close it; When we close a container , The function SCMContainerManager#updateContainerState will remove the container from pipelineContainerMap in SCMPipelineManager.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] github-actions[bot] removed a comment on issue #389:
HDDS-2534. scmcli container delete not working
Posted by GitBox <gi...@apache.org>.
github-actions[bot] removed a comment on issue #389: HDDS-2534. scmcli container delete not working
URL: https://github.com/apache/hadoop-ozone/pull/389#issuecomment-615093872
> /close
Script execution has been failed with exit code 2. Please check the output of the github action run to get more 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#389: HDDS-2534. scmcli container delete not working
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #389: HDDS-2534. scmcli container delete not working
URL: https://github.com/apache/hadoop-ozone/pull/389#discussion_r366605305
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/SCMContainerManager.java
##########
@@ -282,6 +290,12 @@ public ContainerInfo allocateContainer(final ReplicationType type,
public void deleteContainer(ContainerID containerID) throws IOException {
lock.lock();
try {
+ Set<ContainerReplica> replicas =
+ containerStateManager.getContainerReplicas(containerID);
+ for(ContainerReplica replica : replicas){
+ nodeManager.removeContainers(replica.getDatanodeDetails(), Stream.of(
Review comment:
Can we create the set of containerID outside the loop with a simpler Java APIs:
Set<ContainerID> containerIdSet = Collections.singleton(containerID);
replias.forEach(replica->replica.removeContainers(replica.getDatanodeDetails(), containerIdSet));
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] github-actions[bot] commented on issue #389:
HDDS-2534. scmcli container delete not working
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on issue #389: HDDS-2534. scmcli container delete not working
URL: https://github.com/apache/hadoop-ozone/pull/389#issuecomment-615093872
> /close
Script execution has been failed with exit code 2. Please check the output of the github action run to get more 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] elek edited a comment on issue #389: HDDS-2534.
scmcli container delete not working
Posted by GitBox <gi...@apache.org>.
elek edited a comment on issue #389: HDDS-2534. scmcli container delete not working
URL: https://github.com/apache/hadoop-ozone/pull/389#issuecomment-597084180
/pending Questions/suggestions from @xiaoyuyao are not yet addressed in the last commit.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request
#389: HDDS-2534. scmcli container delete not working
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #389: HDDS-2534. scmcli container delete not working
URL: https://github.com/apache/hadoop-ozone/pull/389#discussion_r366606214
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/SCMContainerManager.java
##########
@@ -293,6 +307,12 @@ public void deleteContainer(ContainerID containerID) throws IOException {
" it's missing!", containerID);
}
scmContainerManagerMetrics.incNumSuccessfulDeleteContainers();
+ } catch (NodeNotFoundException nnfe) {
+ scmContainerManagerMetrics.incNumFailureDeleteContainers();
+ throw new SCMException(
+ "Failed to delete container " + containerID + ", reason : " +
+ "node doesn't exist.",
Review comment:
Can we add the node info to the Exception? Is it part of the NodeNotFoundException?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] avijayanhwx commented on a change in pull request
#389: HDDS-2534. scmcli container delete not working
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #389: HDDS-2534. scmcli container delete not working
URL: https://github.com/apache/hadoop-ozone/pull/389#discussion_r367111654
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerReportHandler.java
##########
@@ -119,21 +127,33 @@ public void onMessage(final ContainerReportFromDatanode reportFromDatanode,
* @param datanodeDetails Datanode from which this report was received
* @param replicas list of ContainerReplicaProto
*/
- private void processContainerReplicas(final DatanodeDetails datanodeDetails,
- final List<ContainerReplicaProto> replicas) {
+ private List<ContainerReplicaProto> processContainerReplicas(
+ final DatanodeDetails datanodeDetails,
+ final List<ContainerReplicaProto> replicas,
+ final EventPublisher publisher) {
+ List<ContainerReplicaProto> containers = new ArrayList<>();
for (ContainerReplicaProto replicaProto : replicas) {
try {
processContainerReplica(datanodeDetails, replicaProto);
+ containers.add(replicaProto);
} catch (ContainerNotFoundException e) {
- LOG.error("Received container report for an unknown container" +
- " {} from datanode {}.", replicaProto.getContainerID(),
- datanodeDetails, e);
+ LOG.info("Received container report for an unknown container" +
+ " {} from datanode {},delete it.",
+ replicaProto.getContainerID(),
+ datanodeDetails);
+ publisher.fireEvent(
Review comment:
+1 for @xiaoyuyao's description request. The delete container flow is quite important for Recon to work correctly and hence trying to understand this better. Will there be a case where a user will need to delete a container directly using SCM?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org