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/12/19 20:43:38 UTC
[GitHub] [ozone] nishitpatira opened a new pull request, #4110: HDDS-5560. Updating clusterId to correct value
nishitpatira opened a new pull request, #4110:
URL: https://github.com/apache/ozone/pull/4110
## What changes were proposed in this pull request?
ClusterId was being incorrectly set as datanodeId in the dnlayoutversion/version file.
While this field was not being used anywhere and poses no immediate problem, we wanted to fix it to reflect the correct value.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5560
## How was this patch tested?
Built successfully.
Local test on docker to see the value is being updated correctly.
--
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 #4110: HDDS-5560. Updating clusterId to correct value
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4110:
URL: https://github.com/apache/ozone/pull/4110#issuecomment-1458598141
This PR is waiting for a test case to be added. Instead of marking it as `pending` and getting it auto-closed, I'd like to ask if @DaveTeng0 or @neils-dev would like to contribute the test case?
--
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 #4110: HDDS-5560. Updating clusterId to correct value
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4110:
URL: https://github.com/apache/ozone/pull/4110#issuecomment-1517524409
@sumitagrawl can you please review 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] adoroszlai commented on pull request #4110: HDDS-5560. Updating clusterId to correct value
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4110:
URL: https://github.com/apache/ozone/pull/4110#issuecomment-1462732094
@DaveTeng0 can you please review @neils-dev's updates in https://github.com/apache/ozone/commit/63cdd7c4dafe7e0f1f95b8b555e23f51db47001e?
--
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] neils-dev commented on pull request #4110: HDDS-5560. Updating clusterId to correct value
Posted by "neils-dev (via GitHub)" <gi...@apache.org>.
neils-dev commented on PR #4110:
URL: https://github.com/apache/ozone/pull/4110#issuecomment-1461225122
Thanks @adoroszlai for tending to the status of this PR. I've gone ahead and implemented a test for it. The PR patch however needs a bit of work, upon closer look I found an exception raised repeatedly with the current impl.
```
ozone-datanode-1 | java.io.IOException: Storage directory /data/metadata/dnlayoutversion already initialized.
ozone-datanode-1 | at org.apache.hadoop.ozone.common.Storage.setClusterId(Storage.java:123)
ozone-datanode-1 | at org.apache.hadoop.ozone.container.common.states.endpoint.VersionEndpointTask.call(VersionEndpointTask.java:81)
ozone-datanode-1 | at org.apache.hadoop.ozone.container.common.states.endpoint.VersionEndpointTask.call(VersionEndpointTask.java:41)
ozone-datanode-1 | at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
ozone-datanode-1 | at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
ozone-datanode-1 | at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
ozone-datanode-1 | at java.base/java.lang.Thread.run(Thread.java:829)
ozone-datanode-3 | 2023-03-09 03:41:28,013 [EndpointStateMachine task thread for scm/172.20.0.5:9861 - 0 ] WARN statemachine.EndpointStateMachine: Unable to communicate to SCM server at scm:9861 for past 350 seconds.
```
Needs a little work. I'll push an update when resolved.
--
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 closed pull request #4110: HDDS-5560. Updating clusterId to correct value
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai closed pull request #4110: HDDS-5560. Updating clusterId to correct value
URL: https://github.com/apache/ozone/pull/4110
--
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 #4110: HDDS-5560. Updating clusterId to correct value
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4110:
URL: https://github.com/apache/ozone/pull/4110#issuecomment-1719008733
Continued in #5289.
--
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] neils-dev commented on pull request #4110: HDDS-5560. Updating clusterId to correct value
Posted by "neils-dev (via GitHub)" <gi...@apache.org>.
neils-dev commented on PR #4110:
URL: https://github.com/apache/ozone/pull/4110#issuecomment-1461275000
thanks @adoroszlai . Created a unit test, `TestEndPoint#testDnLayoutVersionFile`. Currently in my branch linked to this PR, see
https://github.com/apache/ozone/compare/master...neils-dev:ozone:pr/4110.
Pls take a look and we can add 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] ashishkumar50 commented on pull request #4110: HDDS-5560. Updating clusterId to correct value
Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.
ashishkumar50 commented on PR #4110:
URL: https://github.com/apache/ozone/pull/4110#issuecomment-1709477534
@neils-dev @nishitpatira If test case is added Can you please merge and close this PR.
--
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 #4110: HDDS-5560. Updating clusterId to correct value
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4110:
URL: https://github.com/apache/ozone/pull/4110#issuecomment-1517532786
@neils-dev thanks for the unit test, I've added some comments on 63cdd7c4dafe7e0f1f95b8b555e23f51db47001e.
--
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] neils-dev commented on pull request #4110: HDDS-5560. Updating clusterId to correct value
Posted by "neils-dev (via GitHub)" <gi...@apache.org>.
neils-dev commented on PR #4110:
URL: https://github.com/apache/ozone/pull/4110#issuecomment-1408120545
Is there a way we can verify if this update to the `dnlayoutversion` file does not affect older versions of ozone, 2.1, 1? @errose28 is this something we need to do?
--
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] neils-dev commented on a diff in pull request #4110: HDDS-5560. Updating clusterId to correct value
Posted by "neils-dev (via GitHub)" <gi...@apache.org>.
neils-dev commented on code in PR #4110:
URL: https://github.com/apache/ozone/pull/4110#discussion_r1096439402
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/DatanodeLayoutStorage.java:
##########
@@ -64,6 +64,11 @@ protected Properties getNodeProperties() {
return new Properties();
}
+ @Override
+ public void setClusterId(String clusterId) throws IOException {
+ super.getStorageInfo().setClusterId(clusterId);
Review Comment:
Thanks @nishitpatira for this patch. We don't need to override the `setClusterId` method here. There is already an implementation provided by the `abstract Storage class`. Not needed here, just use the base class implementation.
--
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] nishitpatira commented on pull request #4110: HDDS-5560. Updating clusterId to correct value
Posted by "nishitpatira (via GitHub)" <gi...@apache.org>.
nishitpatira commented on PR #4110:
URL: https://github.com/apache/ozone/pull/4110#issuecomment-1406474559
@neils-dev @errose28 Can we please review 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] neils-dev commented on pull request #4110: HDDS-5560. Updating clusterId to correct value
Posted by "neils-dev (via GitHub)" <gi...@apache.org>.
neils-dev commented on PR #4110:
URL: https://github.com/apache/ozone/pull/4110#issuecomment-1408119119
Thanks @nishitpatira for submitting this PR for fixing the dnlayoutversion file to properly container the clusterID. I;ve applied your patch and brought up the dev docker cluster. I verified the clusterID for the dnlayoutversion file is the same as the the clusterID marked in the hdds VERSION file as follows,
```
$ docker-compose exec datanode bash
$ cat /data/metadata/dnlayoutversion/VERSION
$ cat /data/hdds/hdds/VERSION
```
Would be great if you would update similar steps to test under the "How patch was tested section". 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] DaveTeng0 commented on pull request #4110: HDDS-5560. Updating clusterId to correct value
Posted by "DaveTeng0 (via GitHub)" <gi...@apache.org>.
DaveTeng0 commented on PR #4110:
URL: https://github.com/apache/ozone/pull/4110#issuecomment-1427589866
Hey @nishitpatira ~ there were some comments from the reviewer for this PR. Please help take a look again when you have time, 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