You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2020/08/28 10:24:08 UTC
[GitHub] [hadoop] jianghuazhu opened a new pull request #2256: HDFS-15546. Remove duplicate initializeGenericKeys method when creating DFSZKFailoverController
jianghuazhu opened a new pull request #2256:
URL: https://github.com/apache/hadoop/pull/2256
…ng DFSZKFailoverController
## NOTICE
Please create an issue in ASF JIRA before opening a pull request,
and you need to set the title of the pull request which starts with
the corresponding JIRA issue number. (e.g. HADOOP-XXXXX. Fix a typo in YYY.)
For more details, please see https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] sunchao commented on a change in pull request #2256: HDFS-15546. Remove duplicate initializeGenericKeys method when creating DFSZKFailoverController
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #2256:
URL: https://github.com/apache/hadoop/pull/2256#discussion_r479379964
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSZKFailoverController.java
##########
@@ -158,7 +158,7 @@ public static DFSZKFailoverController create(Configuration conf) {
"You may run zkfc on the node other than namenode.";
throw new HadoopIllegalArgumentException(msg);
}
- NameNode.initializeGenericKeys(localNNConf, nsId, nnId);
Review comment:
In `NNHAServiceTarget` the call is on a copy of `localNNConf` though so the original conf will not be set.
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/TestDFSZKFailoverController.java
##########
@@ -226,9 +228,11 @@ public void testWithBindAddressSet() throws Exception {
DFSZKFailoverController zkfc = DFSZKFailoverController.create(
conf);
String addr = zkfc.getRpcAddressToBindTo().getHostString();
+ String nameserviceId = conf.get(DFS_NAMESERVICE_ID);
assertEquals("Bind address " + addr + " is not wildcard.",
addr, WILDCARD_ADDRESS);
+ assertEquals(nameserviceId, NAMESERVICE_ID);
Review comment:
nit: `assertEquals(expected, actual)`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] jianghuazhu commented on a change in pull request #2256: HDFS-15546. Remove duplicate initializeGenericKeys method when creating DFSZKFailoverController
Posted by GitBox <gi...@apache.org>.
jianghuazhu commented on a change in pull request #2256:
URL: https://github.com/apache/hadoop/pull/2256#discussion_r479424352
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSZKFailoverController.java
##########
@@ -158,7 +158,7 @@ public static DFSZKFailoverController create(Configuration conf) {
"You may run zkfc on the node other than namenode.";
throw new HadoopIllegalArgumentException(msg);
}
- NameNode.initializeGenericKeys(localNNConf, nsId, nnId);
Review comment:
NameNode.initializeGenericKeys(), this method will be called twice. In addition to this, it will be called once when NNHAServiceTarget() is initialized. Therefore, it can be deleted here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #2256: HDFS-15546. Remove duplicate initializeGenericKeys method when creating DFSZKFailoverController
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2256:
URL: https://github.com/apache/hadoop/pull/2256#issuecomment-682590378
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 41s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 36m 27s | trunk passed |
| +1 :green_heart: | compile | 1m 43s | trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 |
| +1 :green_heart: | compile | 1m 21s | trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
| +1 :green_heart: | checkstyle | 0m 53s | trunk passed |
| +1 :green_heart: | mvnsite | 1m 34s | trunk passed |
| +1 :green_heart: | shadedclient | 20m 46s | branch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 1m 20s | trunk passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 |
| +1 :green_heart: | javadoc | 1m 37s | trunk passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
| +0 :ok: | spotbugs | 4m 4s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 4m 1s | trunk passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 25s | the patch passed |
| +1 :green_heart: | compile | 1m 31s | the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 |
| +1 :green_heart: | javac | 1m 31s | the patch passed |
| +1 :green_heart: | compile | 1m 13s | the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
| +1 :green_heart: | javac | 1m 13s | the patch passed |
| -0 :warning: | checkstyle | 0m 51s | hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 12 unchanged - 0 fixed = 13 total (was 12) |
| +1 :green_heart: | mvnsite | 1m 25s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedclient | 16m 9s | patch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 46s | the patch passed with JDK Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 |
| +1 :green_heart: | javadoc | 1m 14s | the patch passed with JDK Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
| +1 :green_heart: | findbugs | 3m 10s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 111m 42s | hadoop-hdfs in the patch passed. |
| +1 :green_heart: | asflicense | 0m 36s | The patch does not generate ASF License warnings. |
| | | 213m 9s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hdfs.server.sps.TestExternalStoragePolicySatisfier |
| | hadoop.hdfs.TestGetFileChecksum |
| | hadoop.hdfs.TestFileChecksumCompositeCrc |
| | hadoop.hdfs.server.balancer.TestBalancerRPCDelay |
| | hadoop.hdfs.TestRollingUpgrade |
| | hadoop.hdfs.TestFileChecksum |
| | hadoop.fs.contract.hdfs.TestHDFSContractMultipartUploader |
| | hadoop.hdfs.security.TestDelegationToken |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2256/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/2256 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux 1c3e217fb50f 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 06793da1001 |
| Default Java | Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.8+10-post-Ubuntu-0ubuntu118.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_265-8u265-b01-0ubuntu2~18.04-b01 |
| checkstyle | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2256/1/artifact/out/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt |
| unit | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2256/1/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2256/1/testReport/ |
| Max. process+thread count | 2906 (vs. ulimit of 5500) |
| modules | C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2256/1/console |
| versions | git=2.17.1 maven=3.6.0 findbugs=4.0.6 |
| Powered by | Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] jianghuazhu commented on a change in pull request #2256: HDFS-15546. Remove duplicate initializeGenericKeys method when creating DFSZKFailoverController
Posted by GitBox <gi...@apache.org>.
jianghuazhu commented on a change in pull request #2256:
URL: https://github.com/apache/hadoop/pull/2256#discussion_r479437316
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSZKFailoverController.java
##########
@@ -158,7 +158,7 @@ public static DFSZKFailoverController create(Configuration conf) {
"You may run zkfc on the node other than namenode.";
throw new HadoopIllegalArgumentException(msg);
}
- NameNode.initializeGenericKeys(localNNConf, nsId, nnId);
Review comment:
I double checked. I approve of your ideas.
thank you very much.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] sunchao commented on a change in pull request #2256: HDFS-15546. Remove duplicate initializeGenericKeys method when creating DFSZKFailoverController
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #2256:
URL: https://github.com/apache/hadoop/pull/2256#discussion_r479426043
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSZKFailoverController.java
##########
@@ -158,7 +158,7 @@ public static DFSZKFailoverController create(Configuration conf) {
"You may run zkfc on the node other than namenode.";
throw new HadoopIllegalArgumentException(msg);
}
- NameNode.initializeGenericKeys(localNNConf, nsId, nnId);
Review comment:
Yes but as I said, the call in `NNHAServiceTarget` is on a copy:
```java
// Make a copy of the conf, and override configs based on the
// target node -- not the node we happen to be running on.
HdfsConfiguration targetConf = new HdfsConfiguration(conf);
NameNode.initializeGenericKeys(targetConf, nsId, nnId);
```
so the original `localNNConf` won't have those set. I'm not sure there will be any side effect by doing 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] jianghuazhu commented on a change in pull request #2256: HDFS-15546. Remove duplicate initializeGenericKeys method when creating DFSZKFailoverController
Posted by GitBox <gi...@apache.org>.
jianghuazhu commented on a change in pull request #2256:
URL: https://github.com/apache/hadoop/pull/2256#discussion_r479424939
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/TestDFSZKFailoverController.java
##########
@@ -226,9 +228,11 @@ public void testWithBindAddressSet() throws Exception {
DFSZKFailoverController zkfc = DFSZKFailoverController.create(
conf);
String addr = zkfc.getRpcAddressToBindTo().getHostString();
+ String nameserviceId = conf.get(DFS_NAMESERVICE_ID);
assertEquals("Bind address " + addr + " is not wildcard.",
addr, WILDCARD_ADDRESS);
+ assertEquals(nameserviceId, NAMESERVICE_ID);
Review comment:
Is this usage wrong?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] jianghuazhu closed pull request #2256: HDFS-15546. Remove duplicate initializeGenericKeys method when creating DFSZKFailoverController
Posted by GitBox <gi...@apache.org>.
jianghuazhu closed pull request #2256:
URL: https://github.com/apache/hadoop/pull/2256
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] sunchao commented on a change in pull request #2256: HDFS-15546. Remove duplicate initializeGenericKeys method when creating DFSZKFailoverController
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #2256:
URL: https://github.com/apache/hadoop/pull/2256#discussion_r479430862
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/TestDFSZKFailoverController.java
##########
@@ -226,9 +228,11 @@ public void testWithBindAddressSet() throws Exception {
DFSZKFailoverController zkfc = DFSZKFailoverController.create(
conf);
String addr = zkfc.getRpcAddressToBindTo().getHostString();
+ String nameserviceId = conf.get(DFS_NAMESERVICE_ID);
assertEquals("Bind address " + addr + " is not wildcard.",
addr, WILDCARD_ADDRESS);
+ assertEquals(nameserviceId, NAMESERVICE_ID);
Review comment:
Yes: `assertEquals(NAMESERVICE_ID, nameserviceId)`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org