You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "guohao-rosicky (via GitHub)" <gi...@apache.org> on 2023/03/20 06:48:54 UTC
[GitHub] [ozone] guohao-rosicky opened a new pull request, #4425: HDDS-8216. EC: OzoneClientConfig is overwritten in ECKeyOutputStream
guohao-rosicky opened a new pull request, #4425:
URL: https://github.com/apache/ozone/pull/4425
## What changes were proposed in this pull request?
OzoneClientConfig is overwritten in ECKeyOutputStream
see: https://issues.apache.org/jira/browse/HDDS-8216
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-8216
--
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] guohao-rosicky commented on pull request #4425: HDDS-8216. EC: OzoneClientConfig is overwritten in ECKeyOutputStream
Posted by "guohao-rosicky (via GitHub)" <gi...@apache.org>.
guohao-rosicky commented on PR #4425:
URL: https://github.com/apache/ozone/pull/4425#issuecomment-1476270563
> Thanks @guohao-rosicky for updating the patch.
>
> Please try to avoid force-push when updating the PR. Here are some great articles that explain why:
>
> https://developers.mattermost.com/blog/submitting-great-prs/#4-avoid-force-pushing https://www.freecodecamp.org/news/optimize-pull-requests-for-reviewer-happiness#request-a-review
Thank you. That very helpful for 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
[GitHub] [ozone] adoroszlai commented on a diff in pull request #4425: HDDS-8216. EC: OzoneClientConfig is overwritten in ECKeyOutputStream
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #4425:
URL: https://github.com/apache/ozone/pull/4425#discussion_r1141780148
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECKeyOutputStream.java:
##########
@@ -118,7 +118,7 @@ public long getFlushCheckpoint() {
private ECKeyOutputStream(Builder builder) {
super(builder.getReplicationConfig(), builder.getClientMetrics());
- this.config = builder.getClientConfig();
+ this.config = builder.getClientConfig().cloneSelf();
Review Comment:
`clientConfig` is passed to the builder by `RpcClient` here:
https://github.com/apache/ozone/blob/8202b4184e57dede01f21bc5be5563ae85d1dc5b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java#L2257
It should create a new object using the existing method:
```java
.setConfig(conf.getObject(OzoneClientConfig.class))
```
And apply the same fix in other places where `clientConfig` is passed similarly:
https://github.com/apache/ozone/blob/8202b4184e57dede01f21bc5be5563ae85d1dc5b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java#L1796
https://github.com/apache/ozone/blob/8202b4184e57dede01f21bc5be5563ae85d1dc5b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java#L2187
--
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] guohao-rosicky commented on a diff in pull request #4425: HDDS-8216. EC: OzoneClientConfig is overwritten in ECKeyOutputStream
Posted by "guohao-rosicky (via GitHub)" <gi...@apache.org>.
guohao-rosicky commented on code in PR #4425:
URL: https://github.com/apache/ozone/pull/4425#discussion_r1141986260
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECKeyOutputStream.java:
##########
@@ -118,7 +118,7 @@ public long getFlushCheckpoint() {
private ECKeyOutputStream(Builder builder) {
super(builder.getReplicationConfig(), builder.getClientMetrics());
- this.config = builder.getClientConfig();
+ this.config = builder.getClientConfig().cloneSelf();
Review Comment:
Yes, this is better. @adoroszlai 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] guohao-rosicky commented on pull request #4425: HDDS-8216. EC: OzoneClientConfig is overwritten in ECKeyOutputStream
Posted by "guohao-rosicky (via GitHub)" <gi...@apache.org>.
guohao-rosicky commented on PR #4425:
URL: https://github.com/apache/ozone/pull/4425#issuecomment-1475707767
hi, @captainzmc 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] adoroszlai merged pull request #4425: HDDS-8216. EC: OzoneClientConfig is overwritten in ECKeyOutputStream
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #4425:
URL: https://github.com/apache/ozone/pull/4425
--
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