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/08/11 14:11:31 UTC
[GitHub] [ozone] JacksonYao287 opened a new pull request #2525: HDDS-5607. remove container manager v1 code
JacksonYao287 opened a new pull request #2525:
URL: https://github.com/apache/ozone/pull/2525
## What changes were proposed in this pull request?
remove container manager v1 code
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5607
## How was this patch tested?
unit test
--
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: issues-unsubscribe@ozone.apache.org
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] bharatviswa504 commented on a change in pull request #2525: HDDS-5607. remove container manager v1 code
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2525:
URL: https://github.com/apache/ozone/pull/2525#discussion_r711622194
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/container/TestContainerStateManagerIntegration.java
##########
@@ -102,7 +100,7 @@ public void testAllocateContainer() throws IOException {
SCMTestUtils.getReplicationFactor(conf), OzoneConsts.OZONE);
ContainerInfo info = containerManager
.getMatchingContainer(OzoneConsts.GB * 3, OzoneConsts.OZONE,
- container1.getPipeline());
+ container1.getPipeline(), excludedContainerIDS);
Review comment:
We still have old method which is defined in interface ContainerManager.java
default ContainerInfo getMatchingContainer(long size, String owner,
Pipeline pipeline) {
return getMatchingContainer(size, owner, pipeline, Collections.emptySet());
}
But I think that is fine, as anyway for tests we are passing empty hashset for excludedContainerIDS
--
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: issues-unsubscribe@ozone.apache.org
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] JacksonYao287 commented on pull request #2525: HDDS-5607. remove container manager v1 code
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on pull request #2525:
URL: https://github.com/apache/ozone/pull/2525#issuecomment-901064019
@bshashikant PTAL!
--
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: issues-unsubscribe@ozone.apache.org
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] JacksonYao287 commented on pull request #2525: HDDS-5607. remove container manager v1 code
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on pull request #2525:
URL: https://github.com/apache/ozone/pull/2525#issuecomment-901064019
@bshashikant PTAL!
--
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: issues-unsubscribe@ozone.apache.org
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] JacksonYao287 commented on a change in pull request #2525: HDDS-5607. remove container manager v1 code
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on a change in pull request #2525:
URL: https://github.com/apache/ozone/pull/2525#discussion_r705244501
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/container/TestContainerStateManagerIntegration.java
##########
@@ -102,7 +100,7 @@ public void testAllocateContainer() throws IOException {
SCMTestUtils.getReplicationFactor(conf), OzoneConsts.OZONE);
ContainerInfo info = containerManager
.getMatchingContainer(OzoneConsts.GB * 3, OzoneConsts.OZONE,
- container1.getPipeline());
+ container1.getPipeline(), excludedContainerIDS);
Review comment:
```
//V1 code , defined in ContainerManager.java
ContainerInfo getMatchingContainer(long size, String owner,
Pipeline pipeline);
//V2 code, defined in ContainerManagerV2.java
ContainerInfo getMatchingContainer(long size, String owner,
Pipeline pipeline,
Set<ContainerID> excludedContainerIDS);
```
v1 code is not used any more, so after we remove v1 code, we could used V2 method instead.
--
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: issues-unsubscribe@ozone.apache.org
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] bharatviswa504 commented on a change in pull request #2525: HDDS-5607. remove container manager v1 code
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2525:
URL: https://github.com/apache/ozone/pull/2525#discussion_r705208725
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/container/TestContainerStateManagerIntegration.java
##########
@@ -102,7 +100,7 @@ public void testAllocateContainer() throws IOException {
SCMTestUtils.getReplicationFactor(conf), OzoneConsts.OZONE);
ContainerInfo info = containerManager
.getMatchingContainer(OzoneConsts.GB * 3, OzoneConsts.OZONE,
- container1.getPipeline());
+ container1.getPipeline(), excludedContainerIDS);
Review comment:
We still have older method with out needing to pass excludedContainerIDS right?
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineUtils.java
##########
@@ -99,24 +99,6 @@ static void destroyPipeline(DatanodeDetails dn, PipelineID pipelineID,
}
}
- /**
- * Return the list of pipelines who share the same set of datanodes
- * with the input pipeline.
- *
- * @param stateManager PipelineStateManager
- * @param pipeline input pipeline
- * @return list of matched pipeline
- */
- static List<Pipeline> checkPipelineContainSameDatanodes(
- PipelineStateManager stateManager, Pipeline pipeline) {
Review comment:
Is this not required?
--
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: issues-unsubscribe@ozone.apache.org
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] bharatviswa504 merged pull request #2525: HDDS-5607. remove container manager v1 code
Posted by GitBox <gi...@apache.org>.
bharatviswa504 merged pull request #2525:
URL: https://github.com/apache/ozone/pull/2525
--
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: issues-unsubscribe@ozone.apache.org
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] bharatviswa504 commented on pull request #2525: HDDS-5607. remove container manager v1 code
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #2525:
URL: https://github.com/apache/ozone/pull/2525#issuecomment-915972937
Thank You @JacksonYao287 overall LGTM, few minor comments/questions
--
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: issues-unsubscribe@ozone.apache.org
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] JacksonYao287 edited a comment on pull request #2525: HDDS-5607. remove container manager v1 code
Posted by GitBox <gi...@apache.org>.
JacksonYao287 edited a comment on pull request #2525:
URL: https://github.com/apache/ozone/pull/2525#issuecomment-901064019
@bshashikant @ChenSammi PTAL!
--
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: issues-unsubscribe@ozone.apache.org
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] JacksonYao287 commented on a change in pull request #2525: HDDS-5607. remove container manager v1 code
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on a change in pull request #2525:
URL: https://github.com/apache/ozone/pull/2525#discussion_r705237596
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineUtils.java
##########
@@ -99,24 +99,6 @@ static void destroyPipeline(DatanodeDetails dn, PipelineID pipelineID,
}
}
- /**
- * Return the list of pipelines who share the same set of datanodes
- * with the input pipeline.
- *
- * @param stateManager PipelineStateManager
- * @param pipeline input pipeline
- * @return list of matched pipeline
- */
- static List<Pipeline> checkPipelineContainSameDatanodes(
- PipelineStateManager stateManager, Pipeline pipeline) {
Review comment:
yes, it seems not used. for now , we use the following one instead
```
static List<Pipeline> checkPipelineContainSameDatanodes(
StateManager stateManager, Pipeline pipeline) {
```
--
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: issues-unsubscribe@ozone.apache.org
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] JacksonYao287 commented on a change in pull request #2525: HDDS-5607. remove container manager v1 code
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on a change in pull request #2525:
URL: https://github.com/apache/ozone/pull/2525#discussion_r705244501
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/container/TestContainerStateManagerIntegration.java
##########
@@ -102,7 +100,7 @@ public void testAllocateContainer() throws IOException {
SCMTestUtils.getReplicationFactor(conf), OzoneConsts.OZONE);
ContainerInfo info = containerManager
.getMatchingContainer(OzoneConsts.GB * 3, OzoneConsts.OZONE,
- container1.getPipeline());
+ container1.getPipeline(), excludedContainerIDS);
Review comment:
```
//V1 code
ContainerInfo getMatchingContainer(long size, String owner,
Pipeline pipeline);
//V2 code
ContainerInfo getMatchingContainer(long size, String owner,
Pipeline pipeline,
Set<ContainerID> excludedContainerIDS);
```
v1 code is not used any more, so after we remove v1 code, we could used V2 method instead.
--
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: issues-unsubscribe@ozone.apache.org
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] bharatviswa504 commented on pull request #2525: HDDS-5607. remove container manager v1 code
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #2525:
URL: https://github.com/apache/ozone/pull/2525#issuecomment-922349419
Thank You @JacksonYao287 for the contribution.
--
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: issues-unsubscribe@ozone.apache.org
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] JacksonYao287 edited a comment on pull request #2525: HDDS-5607. remove container manager v1 code
Posted by GitBox <gi...@apache.org>.
JacksonYao287 edited a comment on pull request #2525:
URL: https://github.com/apache/ozone/pull/2525#issuecomment-901064019
@bshashikant @ChenSammi @bharatviswa504 PTAL!
--
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: issues-unsubscribe@ozone.apache.org
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