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 2022/05/26 03:59:57 UTC
[GitHub] [ozone] umamaheswararao opened a new pull request, #3458: HDDS-6764: EC: DN ability to create RECOVERING containers for EC reconstruction.
umamaheswararao opened a new pull request, #3458:
URL: https://github.com/apache/ozone/pull/3458
## What changes were proposed in this pull request?
This PR proposes to add the createREcoveringContainer API in ContainerProtocolCalls, which can be used to store reconstruction in progress containers.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-6764
## How was this patch tested?
Added tests to verify.
--
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] adoroszlai commented on a diff in pull request #3458: HDDS-6764: EC: DN ability to create RECOVERING containers for EC reconstruction.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on code in PR #3458:
URL: https://github.com/apache/ozone/pull/3458#discussion_r887228323
##########
hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/container/ContainerTestHelper.java:
##########
@@ -436,6 +441,7 @@ public static Builder newPutBlockRequestBuilder(Pipeline pipeline,
ContainerProtos.PutBlockRequestProto.Builder putRequest =
ContainerProtos.PutBlockRequestProto.newBuilder();
+ putRequest.setEof(true);
Review Comment:
Why is `eof=true` needed here? I think caller can set it later.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java:
##########
@@ -471,7 +473,9 @@ public void validateContainerCommand(
}
State containerState = container.getContainerState();
- if (!HddsUtils.isReadOnly(msg) && containerState != State.OPEN) {
+ if (!HddsUtils.isReadOnly(msg)
+ && (containerState != State.OPEN
+ && containerState != State.RECOVERING)) {
Review Comment:
Would it make sense to add a method for handling OPEN and RECOVERING state the same way in specific cases? (I'm not sure what a good name for the method might be, though.)
##########
hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto:
##########
@@ -270,6 +271,8 @@ message CreateContainerRequestProto {
repeated KeyValue metadata = 2;
optional ContainerType containerType = 3 [default = KeyValueContainer];
optional int32 replicaIndex = 4;
+ //optional LifeCycleState state = 5;
Review Comment:
Do we need to add this?
--
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] umamaheswararao commented on a diff in pull request #3458: HDDS-6764: EC: DN ability to create RECOVERING containers for EC reconstruction.
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on code in PR #3458:
URL: https://github.com/apache/ozone/pull/3458#discussion_r887443919
##########
hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/container/ContainerTestHelper.java:
##########
@@ -436,6 +441,7 @@ public static Builder newPutBlockRequestBuilder(Pipeline pipeline,
ContainerProtos.PutBlockRequestProto.Builder putRequest =
ContainerProtos.PutBlockRequestProto.newBuilder();
+ putRequest.setEof(true);
Review Comment:
Ah, I think I added this while testing.
--
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] kaijchen commented on pull request #3458: HDDS-6764: EC: DN ability to create RECOVERING containers for EC reconstruction.
Posted by GitBox <gi...@apache.org>.
kaijchen commented on PR #3458:
URL: https://github.com/apache/ozone/pull/3458#issuecomment-1145066199
> cc: @kaijchen
> Should we revert that tests until we figure out the cause?
I hope #3476 fixes the problem. If not, we can add a `@Ignore` tag to it.
--
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] umamaheswararao commented on pull request #3458: HDDS-6764: EC: DN ability to create RECOVERING containers for EC reconstruction.
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on PR #3458:
URL: https://github.com/apache/ozone/pull/3458#issuecomment-1145028813
That tests got added recently. I don't think it's related, but there is some flakeyness in the tests. Tests were added from this HDDS-6808
cc: @kaijchen
Should we revert that tests until we figure out the cause?
--
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] umamaheswararao commented on a diff in pull request #3458: HDDS-6764: EC: DN ability to create RECOVERING containers for EC reconstruction.
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on code in PR #3458:
URL: https://github.com/apache/ozone/pull/3458#discussion_r887444065
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java:
##########
@@ -471,7 +473,9 @@ public void validateContainerCommand(
}
State containerState = container.getContainerState();
- if (!HddsUtils.isReadOnly(msg) && containerState != State.OPEN) {
+ if (!HddsUtils.isReadOnly(msg)
+ && (containerState != State.OPEN
+ && containerState != State.RECOVERING)) {
Review Comment:
Tried to export into a method. Let's see if that make sense.
--
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] umamaheswararao commented on pull request #3458: HDDS-6764: EC: DN ability to create RECOVERING containers for EC reconstruction.
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on PR #3458:
URL: https://github.com/apache/ozone/pull/3458#issuecomment-1144350984
Thanks @adoroszlai for the review. Please check the latest updates.
--
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] umamaheswararao merged pull request #3458: HDDS-6764: EC: DN ability to create RECOVERING containers for EC reconstruction.
Posted by GitBox <gi...@apache.org>.
umamaheswararao merged PR #3458:
URL: https://github.com/apache/ozone/pull/3458
--
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] umamaheswararao commented on a diff in pull request #3458: HDDS-6764: EC: DN ability to create RECOVERING containers for EC reconstruction.
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on code in PR #3458:
URL: https://github.com/apache/ozone/pull/3458#discussion_r887443183
##########
hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/container/ContainerTestHelper.java:
##########
@@ -436,6 +441,7 @@ public static Builder newPutBlockRequestBuilder(Pipeline pipeline,
ContainerProtos.PutBlockRequestProto.Builder putRequest =
ContainerProtos.PutBlockRequestProto.newBuilder();
+ putRequest.setEof(true);
Review Comment:
Ah, I think I added when debugging.
##########
hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto:
##########
@@ -270,6 +271,8 @@ message CreateContainerRequestProto {
repeated KeyValue metadata = 2;
optional ContainerType containerType = 3 [default = KeyValueContainer];
optional int32 replicaIndex = 4;
+ //optional LifeCycleState state = 5;
Review Comment:
Below we have added state. I can remove this. Thanks for pointing.
##########
hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto:
##########
@@ -270,6 +271,8 @@ message CreateContainerRequestProto {
repeated KeyValue metadata = 2;
optional ContainerType containerType = 3 [default = KeyValueContainer];
optional int32 replicaIndex = 4;
+ //optional LifeCycleState state = 5;
Review Comment:
removed it. Below state is there.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java:
##########
@@ -471,7 +473,9 @@ public void validateContainerCommand(
}
State containerState = container.getContainerState();
- if (!HddsUtils.isReadOnly(msg) && containerState != State.OPEN) {
+ if (!HddsUtils.isReadOnly(msg)
+ && (containerState != State.OPEN
+ && containerState != State.RECOVERING)) {
Review Comment:
I tried to export into a method. Let's see if that's make sense.
--
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] umamaheswararao commented on pull request #3458: HDDS-6764: EC: DN ability to create RECOVERING containers for EC reconstruction.
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on PR #3458:
URL: https://github.com/apache/ozone/pull/3458#issuecomment-1145113086
I have just rebased with that change. Let's see. If this reappears, then I will add @Ignore tag.
--
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] umamaheswararao commented on pull request #3458: HDDS-6764: EC: DN ability to create RECOVERING containers for EC reconstruction.
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on PR #3458:
URL: https://github.com/apache/ozone/pull/3458#issuecomment-1145308135
Thanks @adoroszlai for the reviews!
--
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] adoroszlai commented on pull request #3458: HDDS-6764: EC: DN ability to create RECOVERING containers for EC reconstruction.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on PR #3458:
URL: https://github.com/apache/ozone/pull/3458#issuecomment-1145022817
@umamaheswararao it looks like `TestOzoneECClient` is failing consistently in CI.
https://github.com/apache/ozone/runs/6702568728#step:5:1644
https://github.com/apache/ozone/runs/6705930506#step:5:1654
https://github.com/apache/ozone/runs/6709532641#step:5:1654
https://github.com/apache/ozone/runs/6710406479#step:5:1654
https://github.com/apache/ozone/runs/6711102895#step:5:1654
--
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