You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "len548 (via GitHub)" <gi...@apache.org> on 2024/12/04 12:36:24 UTC
[PR] HDDS-7003. Provide ability to reconstruct EC files using read-replicas tool [ozone]
len548 opened a new pull request, #7528:
URL: https://github.com/apache/ozone/pull/7528
## What changes were proposed in this pull request?
The ozone debug read-replicas command currently assumes that its replication is RATIS, and it doesn't handle EC replicated keys.
This change tweaked the read-replica command to work with EC replicated keys too.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7003
## How was this patch tested?
Added robot tests and ran them successfully.
--
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
Re: [PR] HDDS-7003. Tweak read-replicas tool to be compatible with EC replication type [ozone]
Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on code in PR #7528:
URL: https://github.com/apache/ozone/pull/7528#discussion_r1879769587
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java:
##########
@@ -1554,23 +1555,26 @@ public OzoneInputStream getKey(
OmKeyInfo keyInfo = getKeyInfo(volumeName, bucketName, keyName, true);
List<OmKeyLocationInfo> keyLocationInfos
= keyInfo.getLatestVersionLocations().getBlocksLatestVersionOnly();
-
+ ReplicationConfig replicationConfig = keyInfo.getReplicationConfig();
for (OmKeyLocationInfo locationInfo : keyLocationInfos) {
Map<DatanodeDetails, OzoneInputStream> blocks = new HashMap<>();
Pipeline pipelineBefore = locationInfo.getPipeline();
List<DatanodeDetails> datanodes = pipelineBefore.getNodes();
-
Review Comment:
Please try to avoid unnecessary changes, like removing or adding empty lines
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java:
##########
@@ -1615,6 +1621,30 @@ public OzoneInputStream getKey(
return result;
}
+ // Helper method to calculate the internal block length of an EC-replicated key.
Review Comment:
Please add a javadoc for this method instead of the comment, thanks!
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java:
##########
@@ -1594,7 +1598,9 @@ public OzoneInputStream getKey(
.setDataSize(keyInfo.getDataSize())
.setCreationTime(keyInfo.getCreationTime())
.setModificationTime(keyInfo.getModificationTime())
- .setReplicationConfig(keyInfo.getReplicationConfig())
+ .setReplicationConfig(replicationConfig instanceof ECReplicationConfig
+ ? RatisReplicationConfig.getInstance(HddsProtos.ReplicationFactor.ONE)
+ : keyInfo.getReplicationConfig())
Review Comment:
Could you elaborate why did you choose to set the replication factor to ONE and RATIS in case of EC? You can do this in the method javadoc and in the PR description. You can add why and how the length is calculated in case of EC. Thanks!
--
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
Re: [PR] HDDS-7003. Tweak read-replicas tool to be compatible with EC replication type [ozone]
Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on code in PR #7528:
URL: https://github.com/apache/ozone/pull/7528#discussion_r1922445105
##########
hadoop-ozone/dist/src/main/smoketest/debug/ozone-debug-tests-ec3-2.robot:
##########
@@ -0,0 +1,103 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+*** Settings ***
+Documentation Test ozone Debug CLI for EC(3,2) replicated keys
+Library OperatingSystem
+Library Process
+Resource ../lib/os.robot
+Resource ozone-debug.robot
+Test Timeout 5 minute
+Suite Setup Create Volume Bucket
+
+*** Variables ***
+${PREFIX} ${EMPTY}
+${VOLUME} cli-debug-volume${PREFIX}
+${BUCKET} cli-debug-bucket
+${TESTFILE} testfile
+${EC_DATA} 3
+${EC_PARITY} 2
+
+*** Keywords ***
+Create Volume Bucket
+ Execute ozone sh volume create o3://om/${VOLUME}
+ Execute ozone sh bucket create o3://om/${VOLUME}/${BUCKET}
+
+Create EC key
+ [arguments] ${bs} ${count}
+
+ Execute dd if=/dev/urandom of=${TEMP_DIR}/testfile bs=${bs} count=${count}
+ Execute ozone sh key put o3://om/${VOLUME}/${BUCKET}/testfile ${TEMP_DIR}/testfile -r rs-${EC_DATA}-${EC_PARITY}-1024k -t EC
Review Comment:
This `Create EC key` is called in evert test case and the key name (`testfile`) is the same each time? I'm not sure how does this run successfully at all, as we create the key on the same path several times. Could you check 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
Re: [PR] HDDS-7003. Tweak read-replicas tool to be compatible with EC replication type [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #7528:
URL: https://github.com/apache/ozone/pull/7528#issuecomment-2638980104
Thanks @len548 for the patch, @dombizita, @fapifta, @peterxcli, @weimingdiit 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.
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
Re: [PR] HDDS-7003. Tweak read-replicas tool to be compatible with EC replication type [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on code in PR #7528:
URL: https://github.com/apache/ozone/pull/7528#discussion_r1930984867
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java:
##########
@@ -1615,6 +1634,34 @@ public OzoneInputStream getKey(
return result;
}
+ /**
+ * Helper method to calculate the internal block length of an EC-replicated key.
+ * The length of data blocks differs depending on whether it is the last strip or not.
+ * That of parity blocks can be determined by the data blocks.
+ */
+ private long internalBlockLength(int index, ECReplicationConfig ecRepConfig, long blockLength) {
Review Comment:
Should we consider unifying these two methods? It seems critical to ensure consistency, and it might be better to place this logic in a shared location that everyone can access and reuse.
--
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
Re: [PR] HDDS-7003. Tweak read-replicas tool to be compatible with EC replication type [ozone]
Posted by "len548 (via GitHub)" <gi...@apache.org>.
len548 commented on code in PR #7528:
URL: https://github.com/apache/ozone/pull/7528#discussion_r1935421028
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java:
##########
@@ -1615,6 +1634,34 @@ public OzoneInputStream getKey(
return result;
}
+ /**
+ * Helper method to calculate the internal block length of an EC-replicated key.
+ * The length of data blocks differs depending on whether it is the last strip or not.
+ * That of parity blocks can be determined by the data blocks.
+ */
+ private long internalBlockLength(int index, ECReplicationConfig ecRepConfig, long blockLength) {
Review Comment:
It sounds good to me. I modified it to public static method.
--
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
Re: [PR] HDDS-7003. Tweak read-replicas tool to be compatible with EC replication type [ozone]
Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on code in PR #7528:
URL: https://github.com/apache/ozone/pull/7528#discussion_r1930507316
##########
hadoop-ozone/dist/src/main/smoketest/debug/ozone-debug-tests-ec3-2.robot:
##########
@@ -0,0 +1,103 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+*** Settings ***
+Documentation Test ozone Debug CLI for EC(3,2) replicated keys
+Library OperatingSystem
+Library Process
+Resource ../lib/os.robot
+Resource ozone-debug.robot
+Test Timeout 5 minute
+Suite Setup Create Volume Bucket
+
+*** Variables ***
+${PREFIX} ${EMPTY}
+${VOLUME} cli-debug-volume${PREFIX}
+${BUCKET} cli-debug-bucket
+${TESTFILE} testfile
+${EC_DATA} 3
+${EC_PARITY} 2
+
+*** Keywords ***
+Create Volume Bucket
+ Execute ozone sh volume create o3://om/${VOLUME}
+ Execute ozone sh bucket create o3://om/${VOLUME}/${BUCKET}
+
+Create EC key
+ [arguments] ${bs} ${count}
+
+ Execute dd if=/dev/urandom of=${TEMP_DIR}/testfile bs=${bs} count=${count}
+ Execute ozone sh key put o3://om/${VOLUME}/${BUCKET}/testfile ${TEMP_DIR}/testfile -r rs-${EC_DATA}-${EC_PARITY}-1024k -t EC
Review Comment:
Thanks for checking this, you are right, this happens because you are using the object store API with the `ozone sh key put` command.
--
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
Re: [PR] HDDS-7003. Tweak read-replicas tool to be compatible with EC replication type [ozone]
Posted by "weimingdiit (via GitHub)" <gi...@apache.org>.
weimingdiit commented on code in PR #7528:
URL: https://github.com/apache/ozone/pull/7528#discussion_r1914368307
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java:
##########
@@ -1615,6 +1634,34 @@ public OzoneInputStream getKey(
return result;
}
+ /**
+ * Helper method to calculate the internal block length of an EC-replicated key.
+ * The length of data blocks differs depending on whether it is the last strip or not.
+ * That of parity blocks can be determined by the data blocks.
+ */
+ private long internalBlockLength(int index, ECReplicationConfig ecRepConfig, long blockLength) {
Review Comment:
Why do we calculate the block length in this way? Could you explain it in detail? Thanks.
--
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
Re: [PR] HDDS-7003. Tweak read-replicas tool to be compatible with EC replication type [ozone]
Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on PR #7528:
URL: https://github.com/apache/ozone/pull/7528#issuecomment-2615731003
@len548 Please resolve the conflicts, otherwise it looks good to me!
--
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
Re: [PR] HDDS-7003. Tweak read-replicas tool to be compatible with EC replication type [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #7528:
URL: https://github.com/apache/ozone/pull/7528
--
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