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/07/20 14:22:52 UTC
[GitHub] [ozone] elek opened a new pull request #2444: HDDS-5364. EC: Adopt EC related utility from Hadoop source repository
elek opened a new pull request #2444:
URL: https://github.com/apache/ozone/pull/2444
## What changes were proposed in this pull request?
It seems only a few key classes are required from Apache Hadoop to be reused. The goal here is investigating the build process and check how they can be re-used.
One option is just copy the files with some minor updates (e.g. using `ConfigurationSource` interface instead of `o.a.hadoop.Configuration`) which would help to reduce dependencies.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5364
## How was this patch tested?
Full CI
--
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 #2444: HDDS-5364. EC: Adopt EC related utility from Hadoop source repository
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on pull request #2444:
URL: https://github.com/apache/ozone/pull/2444#issuecomment-940509954
Ok, Looks like you are busy. No issues. I am going ahead to take over this work and move forward for the closure. 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
[GitHub] [ozone] umamaheswararao commented on pull request #2444: HDDS-5364. EC: Adopt EC related utility from Hadoop source repository
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on pull request #2444:
URL: https://github.com/apache/ozone/pull/2444#issuecomment-937394478
@elek could you please take a look at the conflicts and comments? If you are busy, request you to un-assign the JIRA, so that someone else can take it forward as co-author with you. 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
[GitHub] [ozone] umamaheswararao commented on pull request #2444: HDDS-5364. EC: Adopt EC related utility from Hadoop source repository
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on pull request #2444:
URL: https://github.com/apache/ozone/pull/2444#issuecomment-949857610
Merged the changed as part of #2733
--
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 #2444: HDDS-5364. EC: Adopt EC related utility from Hadoop source repository
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on pull request #2444:
URL: https://github.com/apache/ozone/pull/2444#issuecomment-900306470
Thanks @elek for the patch. Sorry for the late on this. I am thinking that, we should copy the code as it is in JIRA and then modify in next JIRA? SInce it's large patch it may be hard to follow the changes against hadoop code here.
Thoughts?
CC: @sodonnel @fapifta
--
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 change in pull request #2444: HDDS-5364. EC: Adopt EC related utility from Hadoop source repository
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on a change in pull request #2444:
URL: https://github.com/apache/ozone/pull/2444#discussion_r703608003
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ECReplicationConfig.java
##########
@@ -75,6 +87,7 @@ public int getRequiredNodes() {
return HddsProtos.ECReplicationConfig.newBuilder()
.setData(data)
.setParity(parity)
+ .setCodec(codec)
.build();
}
Review comment:
Should we consider codec also in equals?
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ECReplicationConfig.java
##########
@@ -28,18 +28,29 @@
* Replication configuration for EC replication.
*/
public class ECReplicationConfig implements ReplicationConfig {
-
+
private static final Pattern STRING_FORMAT = Pattern.compile("(\\d+)-(\\d+)");
-
+
+ public static final String RS_CODEC = "rs";
+ public static final String XOR_CODEC = "xor";
+
private int data;
private int parity;
+ private String codec = RS_CODEC;
+
public ECReplicationConfig(int data, int parity) {
this.data = data;
this.parity = parity;
}
+ public ECReplicationConfig(int data, int parity, String codec) {
+ this.data = data;
+ this.parity = parity;
+ this.codec = codec;
+ }
+
public ECReplicationConfig(String string) {
final Matcher matcher = STRING_FORMAT.matcher(string);
if (!matcher.matches()) {
Review comment:
You may want to update this message as we may allow codec also in config?
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ECReplicationConfig.java
##########
@@ -28,18 +28,29 @@
* Replication configuration for EC replication.
*/
public class ECReplicationConfig implements ReplicationConfig {
-
+
private static final Pattern STRING_FORMAT = Pattern.compile("(\\d+)-(\\d+)");
Review comment:
Now we should be able to configure with codec option right? Could you please add comment to show the expected valid format ?
##########
File path: hadoop-hdds/erasurecode/src/main/java/org/apache/ozone/erasurecode/ECChunk.java
##########
@@ -0,0 +1,113 @@
+/**
+ * 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.
+ */
+package org.apache.ozone.erasurecode;
+
+import java.nio.ByteBuffer;
+
+/**
+ * A wrapper for ByteBuffer or bytes array for an erasure code chunk.
+ */
+public class ECChunk {
Review comment:
Looks like ECChunk abstraction we used only in tests? I think it's ok to have it for now and later we can cleanup if we don't use it ( to avoid code modifications in this patch)
--
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 #2444: HDDS-5364. EC: Adopt EC related utility from Hadoop source repository
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on pull request #2444:
URL: https://github.com/apache/ozone/pull/2444#issuecomment-937394478
@elek could you please take a look at the conflicts and comments? If you are busy, request you to un-assign the JIRA, so that someone else can take it forward as co-author with you. 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
[GitHub] [ozone] umamaheswararao closed pull request #2444: HDDS-5364. EC: Adopt EC related utility from Hadoop source repository
Posted by GitBox <gi...@apache.org>.
umamaheswararao closed pull request #2444:
URL: https://github.com/apache/ozone/pull/2444
--
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] elek commented on pull request #2444: HDDS-5364. EC: Adopt EC related utility from Hadoop source repository
Posted by GitBox <gi...@apache.org>.
elek commented on pull request #2444:
URL: https://github.com/apache/ozone/pull/2444#issuecomment-893259508
@sodonnel @umamaheswararao Can you please review 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] elek commented on pull request #2444: HDDS-5364. EC: Adopt EC related utility from Hadoop source repository
Posted by GitBox <gi...@apache.org>.
elek commented on pull request #2444:
URL: https://github.com/apache/ozone/pull/2444#issuecomment-893259508
@sodonnel @umamaheswararao Can you please review 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] elek commented on pull request #2444: HDDS-5364. EC: Adopt EC related utility from Hadoop source repository
Posted by GitBox <gi...@apache.org>.
elek commented on pull request #2444:
URL: https://github.com/apache/ozone/pull/2444#issuecomment-945387457
> I am going ahead to take over this work and move forward for the closure.
Sure, thanks a lot. Please, do 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 #2444: HDDS-5364. EC: Adopt EC related utility from Hadoop source repository
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on pull request #2444:
URL: https://github.com/apache/ozone/pull/2444#issuecomment-914428347
> Thanks the answer @umamaheswararao
>
> However, I am not sure how is it possible. Just copying the code wouldn't compile. We have different dependency structure, package names and configuration objects.
>
> But here I didn't change any logic, just copied the code and updated the used configuration interfaces. So technicaly this patch is the copy without any significant change.
I agree there is no logic change. Only worry is about the patch size and it's easy to miss things. For example, above conflict changes are not necessarily in this patch I guess. They are about using the new EcReplicationConfig with codec option. Currently they are already have default values and we can easily use this patch ECReplicationConfig in next patch. That way we can reduce modifications in this patch.
--
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 #2444: HDDS-5364. EC: Adopt EC related utility from Hadoop source repository
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on pull request #2444:
URL: https://github.com/apache/ozone/pull/2444#issuecomment-919292801
@elek did you get chance to check my comments? 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
[GitHub] [ozone] elek commented on pull request #2444: HDDS-5364. EC: Adopt EC related utility from Hadoop source repository
Posted by GitBox <gi...@apache.org>.
elek commented on pull request #2444:
URL: https://github.com/apache/ozone/pull/2444#issuecomment-906410952
Thanks the answer @umamaheswararao
However, I am not sure how is it possible. Just copying the code wouldn't compile. We have different dependency structure, package names and configuration objects.
But here I didn't change any logic, just copied the code and updated the used configuration interfaces. So technicaly this patch is the copy without any significant change.
--
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