[GitHub] [hadoop] Hexiaoqiao commented on a change in pull request #2037: HDFS-14984. HDFS setQuota: Error message should be added for invalid …
Hexiaoqiao commented on a change in pull request #2037: URL: https://github.com/apache/hadoop/pull/2037#discussion_r449981097 ## File path: hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSClient.java ## @@ -2553,16 +2553,17 @@ void setQuota(String src, long namespaceQuota, long storagespaceQuota) throws IOException { checkOpen(); // sanity check -if ((namespaceQuota <= 0 && - namespaceQuota != HdfsConstants.QUOTA_DONT_SET && - namespaceQuota != HdfsConstants.QUOTA_RESET) || -(storagespaceQuota < 0 && +if (namespaceQuota <= 0 && +namespaceQuota != HdfsConstants.QUOTA_DONT_SET && +namespaceQuota != HdfsConstants.QUOTA_RESET){ + throw new IllegalArgumentException("Invalid values for " + + "namespace quota : " + namespaceQuota); +} +if (storagespaceQuota < 0 && Review comment: I mean if we should update `storagespaceQuota < 0` to `storagespaceQuota <= 0`, rather than whole condition statement. ## File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSAdmin.java ## @@ -201,8 +201,19 @@ public void run(Path path) throws IOException { super(conf); CommandFormat c = new CommandFormat(2, Integer.MAX_VALUE); List parameters = c.parse(args, pos); - this.quota = - StringUtils.TraditionalBinaryPrefix.string2long(parameters.remove(0)); + String str = parameters.get(0).trim(); + try { +this.quota = StringUtils.TraditionalBinaryPrefix +.string2long(parameters.remove(0)); + } catch(NumberFormatException e){ +throw new IllegalArgumentException("\"" + str + +"\" is not a valid value for a quota."); + } + if (HdfsConstants.QUOTA_DONT_SET == this.quota) { +System.out.print("WARN: \"" + this.quota + Review comment: or how about return directly? I think it is not necessary to send RPC request to NameNode if has checked the parameter is invalid. 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: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] zhaoyim commented on a change in pull request #2037: HDFS-14984. HDFS setQuota: Error message should be added for invalid …
zhaoyim commented on a change in pull request #2037: URL: https://github.com/apache/hadoop/pull/2037#discussion_r449973580 ## File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSAdmin.java ## @@ -201,8 +201,19 @@ public void run(Path path) throws IOException { super(conf); CommandFormat c = new CommandFormat(2, Integer.MAX_VALUE); List parameters = c.parse(args, pos); - this.quota = - StringUtils.TraditionalBinaryPrefix.string2long(parameters.remove(0)); + String str = parameters.get(0).trim(); + try { +this.quota = StringUtils.TraditionalBinaryPrefix +.string2long(parameters.remove(0)); + } catch(NumberFormatException e){ +throw new IllegalArgumentException("\"" + str + +"\" is not a valid value for a quota."); + } + if (HdfsConstants.QUOTA_DONT_SET == this.quota) { +System.out.print("WARN: \"" + this.quota + Review comment: Changed to ```System.err.print```. And I agree with you, not need to execute the backward code, but here I confused is this message just a warning message, it is suitable to throw an Exception ? Any ideas? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] zhaoyim commented on a change in pull request #2037: HDFS-14984. HDFS setQuota: Error message should be added for invalid …
zhaoyim commented on a change in pull request #2037: URL: https://github.com/apache/hadoop/pull/2037#discussion_r449973580 ## File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSAdmin.java ## @@ -201,8 +201,19 @@ public void run(Path path) throws IOException { super(conf); CommandFormat c = new CommandFormat(2, Integer.MAX_VALUE); List parameters = c.parse(args, pos); - this.quota = - StringUtils.TraditionalBinaryPrefix.string2long(parameters.remove(0)); + String str = parameters.get(0).trim(); + try { +this.quota = StringUtils.TraditionalBinaryPrefix +.string2long(parameters.remove(0)); + } catch(NumberFormatException e){ +throw new IllegalArgumentException("\"" + str + +"\" is not a valid value for a quota."); + } + if (HdfsConstants.QUOTA_DONT_SET == this.quota) { +System.out.print("WARN: \"" + this.quota + Review comment: Changed to ```System.err.print```. And I agree with you, not need to execute the backward code, but here my confusion is this message just a warning message, it is suitable to throw an Exception ? Any ideas? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] zhaoyim commented on a change in pull request #2037: HDFS-14984. HDFS setQuota: Error message should be added for invalid …
zhaoyim commented on a change in pull request #2037: URL: https://github.com/apache/hadoop/pull/2037#discussion_r449972937 ## File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSAdmin.java ## @@ -323,6 +334,11 @@ public void run(Path path) throws IOException { } catch (NumberFormatException nfe) { throw new IllegalArgumentException("\"" + str + "\" is not a valid value for a quota."); } + if (HdfsConstants.QUOTA_DONT_SET == quota) { +System.out.print("WARN: \"" + this.quota + Review comment: changed to ```System.err.print``` 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: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] zhaoyim commented on a change in pull request #2037: HDFS-14984. HDFS setQuota: Error message should be added for invalid …
zhaoyim commented on a change in pull request #2037: URL: https://github.com/apache/hadoop/pull/2037#discussion_r449972840 ## File path: hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSClient.java ## @@ -2553,16 +2553,17 @@ void setQuota(String src, long namespaceQuota, long storagespaceQuota) throws IOException { checkOpen(); // sanity check -if ((namespaceQuota <= 0 && - namespaceQuota != HdfsConstants.QUOTA_DONT_SET && - namespaceQuota != HdfsConstants.QUOTA_RESET) || -(storagespaceQuota < 0 && +if (namespaceQuota <= 0 && +namespaceQuota != HdfsConstants.QUOTA_DONT_SET && +namespaceQuota != HdfsConstants.QUOTA_RESET){ + throw new IllegalArgumentException("Invalid values for " + + "namespace quota : " + namespaceQuota); +} +if (storagespaceQuota < 0 && Review comment: @Hexiaoqiao Thanks for review! ```(-9223372036854775808L - 1) == Long.MAX_VALUE``` and ```QUOTA_RESET = -1L``` so I think it's better to check more conditions. 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: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] zhaoyim commented on a change in pull request #2037: HDFS-14984. HDFS setQuota: Error message should be added for invalid …
zhaoyim commented on a change in pull request #2037: URL: https://github.com/apache/hadoop/pull/2037#discussion_r449973580 ## File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSAdmin.java ## @@ -201,8 +201,19 @@ public void run(Path path) throws IOException { super(conf); CommandFormat c = new CommandFormat(2, Integer.MAX_VALUE); List parameters = c.parse(args, pos); - this.quota = - StringUtils.TraditionalBinaryPrefix.string2long(parameters.remove(0)); + String str = parameters.get(0).trim(); + try { +this.quota = StringUtils.TraditionalBinaryPrefix +.string2long(parameters.remove(0)); + } catch(NumberFormatException e){ +throw new IllegalArgumentException("\"" + str + +"\" is not a valid value for a quota."); + } + if (HdfsConstants.QUOTA_DONT_SET == this.quota) { +System.out.print("WARN: \"" + this.quota + Review comment: Changed to ```System.err.print```. And I agree with you not need to execute the backward code, but here I confused is this message just a warning message, it is suitable to throw an Exception ? Any ideas? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] zhaoyim commented on a change in pull request #2037: HDFS-14984. HDFS setQuota: Error message should be added for invalid …
zhaoyim commented on a change in pull request #2037: URL: https://github.com/apache/hadoop/pull/2037#discussion_r449972937 ## File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSAdmin.java ## @@ -323,6 +334,11 @@ public void run(Path path) throws IOException { } catch (NumberFormatException nfe) { throw new IllegalArgumentException("\"" + str + "\" is not a valid value for a quota."); } + if (HdfsConstants.QUOTA_DONT_SET == quota) { +System.out.print("WARN: \"" + this.quota + Review comment: changed to System.err 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: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] zhaoyim commented on a change in pull request #2037: HDFS-14984. HDFS setQuota: Error message should be added for invalid …
zhaoyim commented on a change in pull request #2037: URL: https://github.com/apache/hadoop/pull/2037#discussion_r449972840 ## File path: hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSClient.java ## @@ -2553,16 +2553,17 @@ void setQuota(String src, long namespaceQuota, long storagespaceQuota) throws IOException { checkOpen(); // sanity check -if ((namespaceQuota <= 0 && - namespaceQuota != HdfsConstants.QUOTA_DONT_SET && - namespaceQuota != HdfsConstants.QUOTA_RESET) || -(storagespaceQuota < 0 && +if (namespaceQuota <= 0 && +namespaceQuota != HdfsConstants.QUOTA_DONT_SET && +namespaceQuota != HdfsConstants.QUOTA_RESET){ + throw new IllegalArgumentException("Invalid values for " + + "namespace quota : " + namespaceQuota); +} +if (storagespaceQuota < 0 && Review comment: @Hexiaoqiao Thanks for review! ```(-9223372036854775808L - 1) == Long.MAX_VALUE``` and ```QUOTA_RESET = -1L``` so I think it's better to check more condition. 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: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #2080: HDFS-15417. RBF: Get the datanode report from cache for federation WebHDFS operations
hadoop-yetus commented on pull request #2080: URL: https://github.com/apache/hadoop/pull/2080#issuecomment-653990650 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 1m 14s | 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 | 22m 10s | trunk passed | | +1 :green_heart: | compile | 0m 36s | trunk passed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04 | | +1 :green_heart: | compile | 0m 31s | trunk passed with JDK Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | +1 :green_heart: | checkstyle | 0m 21s | trunk passed | | +1 :green_heart: | mvnsite | 0m 36s | trunk passed | | +1 :green_heart: | shadedclient | 16m 36s | branch has no errors when building and testing our client artifacts. | | -1 :x: | javadoc | 0m 28s | hadoop-hdfs-rbf in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | +1 :green_heart: | javadoc | 0m 32s | trunk passed with JDK Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | +0 :ok: | spotbugs | 1m 15s | Used deprecated FindBugs config; considering switching to SpotBugs. | | +1 :green_heart: | findbugs | 1m 13s | trunk passed | ||| _ Patch Compile Tests _ | | +1 :green_heart: | mvninstall | 0m 30s | the patch passed | | +1 :green_heart: | compile | 0m 34s | the patch passed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04 | | +1 :green_heart: | javac | 0m 34s | the patch passed | | +1 :green_heart: | compile | 0m 28s | the patch passed with JDK Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | +1 :green_heart: | javac | 0m 28s | the patch passed | | -0 :warning: | checkstyle | 0m 15s | hadoop-hdfs-project/hadoop-hdfs-rbf: The patch generated 1 new + 16 unchanged - 0 fixed = 17 total (was 16) | | +1 :green_heart: | mvnsite | 0m 29s | the patch passed | | +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. | | +1 :green_heart: | shadedclient | 16m 34s | patch has no errors when building and testing our client artifacts. | | -1 :x: | javadoc | 0m 22s | hadoop-hdfs-rbf in the patch failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | +1 :green_heart: | javadoc | 0m 26s | the patch passed with JDK Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | +1 :green_heart: | findbugs | 1m 18s | the patch passed | ||| _ Other Tests _ | | -1 :x: | unit | 10m 12s | hadoop-hdfs-rbf in the patch passed. | | +1 :green_heart: | asflicense | 0m 28s | The patch does not generate ASF License warnings. | | | | 78m 46s | | | Reason | Tests | |---:|:--| | Failed junit tests | hadoop.hdfs.server.federation.router.TestRouterRpc | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-2080/9/artifact/out/Dockerfile | | GITHUB PR | https://github.com/apache/hadoop/pull/2080 | | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle | | uname | Linux c37581c63ca7 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | personality/hadoop.sh | | git revision | trunk / 55a2ae80dc9 | | Default Java | Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | javadoc | https://builds.apache.org/job/hadoop-multibranch/job/PR-2080/9/artifact/out/branch-javadoc-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkUbuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04.txt | | checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-2080/9/artifact/out/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs-rbf.txt | | javadoc | https://builds.apache.org/job/hadoop-multibranch/job/PR-2080/9/artifact/out/patch-javadoc-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkUbuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04.txt | | unit | https://builds.apache.org/job/hadoop-multibranch/job/PR-2080/9/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt | | Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-2080/9/testReport/ | | Max. process+thread count | 2779
[GitHub] [hadoop] hadoop-yetus commented on pull request #2110: HDFS-15447 RBF: Add top real owners metrics for delegation tokens
hadoop-yetus commented on pull request #2110: URL: https://github.com/apache/hadoop/pull/2110#issuecomment-653976649 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 21m 42s | Docker mode activated. | ||| _ Prechecks _ | | +1 :green_heart: | dupname | 0m 1s | 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 _ | | +0 :ok: | mvndep | 1m 11s | Maven dependency ordering for branch | | +1 :green_heart: | mvninstall | 19m 13s | trunk passed | | +1 :green_heart: | compile | 19m 11s | trunk passed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04 | | +1 :green_heart: | compile | 16m 35s | trunk passed with JDK Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | +1 :green_heart: | checkstyle | 3m 17s | trunk passed | | +1 :green_heart: | mvnsite | 2m 21s | trunk passed | | +1 :green_heart: | shadedclient | 20m 16s | branch has no errors when building and testing our client artifacts. | | -1 :x: | javadoc | 0m 45s | hadoop-common in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 41s | hadoop-hdfs-rbf in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | +1 :green_heart: | javadoc | 1m 48s | trunk passed with JDK Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | +0 :ok: | spotbugs | 1m 21s | Used deprecated FindBugs config; considering switching to SpotBugs. | | +1 :green_heart: | findbugs | 3m 26s | trunk passed | ||| _ Patch Compile Tests _ | | +0 :ok: | mvndep | 0m 26s | Maven dependency ordering for patch | | +1 :green_heart: | mvninstall | 1m 20s | the patch passed | | +1 :green_heart: | compile | 18m 25s | the patch passed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04 | | +1 :green_heart: | javac | 18m 25s | the patch passed | | +1 :green_heart: | compile | 16m 36s | the patch passed with JDK Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | +1 :green_heart: | javac | 16m 36s | the patch passed | | -0 :warning: | checkstyle | 2m 41s | root: The patch generated 1 new + 60 unchanged - 2 fixed = 61 total (was 62) | | +1 :green_heart: | mvnsite | 2m 17s | the patch passed | | +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. | | +1 :green_heart: | xml | 0m 1s | The patch has no ill-formed XML file. | | +1 :green_heart: | shadedclient | 14m 8s | patch has no errors when building and testing our client artifacts. | | -1 :x: | javadoc | 0m 44s | hadoop-common in the patch failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 41s | hadoop-hdfs-rbf in the patch failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | +1 :green_heart: | javadoc | 1m 48s | the patch passed with JDK Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | +1 :green_heart: | findbugs | 3m 42s | the patch passed | ||| _ Other Tests _ | | +1 :green_heart: | unit | 9m 29s | hadoop-common in the patch passed. | | +1 :green_heart: | unit | 8m 15s | hadoop-hdfs-rbf in the patch passed. | | +1 :green_heart: | asflicense | 0m 55s | The patch does not generate ASF License warnings. | | | | 191m 42s | | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-2110/4/artifact/out/Dockerfile | | GITHUB PR | https://github.com/apache/hadoop/pull/2110 | | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle xml | | uname | Linux a23c7d24a34e 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | personality/hadoop.sh | | git revision | trunk / 55a2ae80dc9 | | Default Java | Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | javadoc | https://builds.apache.org/job/hadoop-multibranch/job/PR-2110/4/artifact/out/branch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04.txt | | javadoc | https://builds.apache.org/job/hadoop-multibranch/job/PR-2110/4/artifact/out/branch-javadoc-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkUbuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04.txt |
[GitHub] [hadoop] hadoop-yetus commented on pull request #2122: [HADOOP-17109] Replace Guava base64Url and base64 with Java8+ base64
hadoop-yetus commented on pull request #2122: URL: https://github.com/apache/hadoop/pull/2122#issuecomment-653956065 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 1m 32s | Docker mode activated. | ||| _ Prechecks _ | | +1 :green_heart: | dupname | 0m 1s | 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 14 new or modified test files. | ||| _ trunk Compile Tests _ | | +0 :ok: | mvndep | 0m 27s | Maven dependency ordering for branch | | +1 :green_heart: | mvninstall | 24m 39s | trunk passed | | +1 :green_heart: | compile | 24m 38s | trunk passed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04 | | +1 :green_heart: | compile | 20m 41s | trunk passed with JDK Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | +1 :green_heart: | checkstyle | 3m 15s | trunk passed | | +1 :green_heart: | mvnsite | 15m 28s | trunk passed | | +1 :green_heart: | shadedclient | 36m 43s | branch has no errors when building and testing our client artifacts. | | -1 :x: | javadoc | 0m 35s | hadoop-auth in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 40s | hadoop-common in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 31s | hadoop-kms in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 34s | hadoop-registry in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 47s | hadoop-hdfs-client in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 45s | hadoop-hdfs in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 37s | hadoop-hdfs-rbf in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 38s | hadoop-yarn-common in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 36s | hadoop-yarn-server-nodemanager in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 39s | hadoop-yarn-server-resourcemanager in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 34s | hadoop-yarn-client in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 40s | hadoop-aws in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 35s | hadoop-azure in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 33s | hadoop-cos in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | +1 :green_heart: | javadoc | 10m 41s | trunk passed with JDK Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | +0 :ok: | spotbugs | 0m 55s | Used deprecated FindBugs config; considering switching to SpotBugs. | | -1 :x: | findbugs | 0m 52s | hadoop-cloud-storage-project/hadoop-cos in trunk has 1 extant findbugs warnings. | ||| _ Patch Compile Tests _ | | +0 :ok: | mvndep | 0m 25s | Maven dependency ordering for patch | | +1 :green_heart: | mvninstall | 11m 8s | the patch passed | | +1 :green_heart: | compile | 21m 26s | the patch passed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04 | | -1 :x: | javac | 21m 26s | root-jdkUbuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04 with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04 generated 2 new + 1975 unchanged - 2 fixed = 1977 total (was 1977) | | +1 :green_heart: | compile | 17m 34s | the patch passed with JDK Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | -1 :x: | javac | 17m 34s | root-jdkPrivateBuild-1.8.0_252-8u252-b09-1~18.04-b09 with JDK Private Build-1.8.0_252-8u252-b09-1~18.04-b09 generated 2 new + 1868 unchanged - 2 fixed = 1870 total (was 1870) | | -0 :warning: | checkstyle | 2m 55s | root: The patch generated 20 new + 389 unchanged - 7 fixed = 409 total (was 396) | | +1 :green_heart: | mvnsite | 13m 29s | the patch passed | | +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. | | +1 :green_heart: | shadedclient | 15m 43s | patch has no errors when building and testing our client artifacts. | | -1 :x: | javadoc | 0m 31s | hadoop-auth in the patch failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 37s | hadoop-common in the patch failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. |
[GitHub] [hadoop] fengnanli commented on pull request #2110: HDFS-15447 RBF: Add top real owners metrics for delegation tokens
fengnanli commented on pull request #2110: URL: https://github.com/apache/hadoop/pull/2110#issuecomment-653946194 Thanks very much @sunchao @goiri for the detailed review. I have addressed all comments and please give it another look. 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: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #2096: HDFS-15312. Apply umask when creating directory by WebHDFS
hadoop-yetus commented on pull request #2096: URL: https://github.com/apache/hadoop/pull/2096#issuecomment-653945068 :confetti_ball: **+1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 0m 30s | Docker mode activated. | ||| _ Prechecks _ | | +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. | | +0 :ok: | jshint | 0m 0s | jshint was not available. | | +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. | ||| _ trunk Compile Tests _ | | +0 :ok: | mvndep | 1m 2s | Maven dependency ordering for branch | | +1 :green_heart: | mvninstall | 18m 56s | trunk passed | | +1 :green_heart: | shadedclient | 33m 59s | branch has no errors when building and testing our client artifacts. | ||| _ Patch Compile Tests _ | | +0 :ok: | mvndep | 0m 25s | Maven dependency ordering for patch | | +1 :green_heart: | mvninstall | 1m 38s | the patch passed | | +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. | | +1 :green_heart: | shadedclient | 14m 2s | patch has no errors when building and testing our client artifacts. | ||| _ Other Tests _ | | +1 :green_heart: | asflicense | 0m 32s | The patch does not generate ASF License warnings. | | | | 52m 52s | | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-2096/3/artifact/out/Dockerfile | | GITHUB PR | https://github.com/apache/hadoop/pull/2096 | | Optional Tests | dupname asflicense shadedclient jshint | | uname | Linux 97a089584bbd 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | personality/hadoop.sh | | git revision | trunk / 55a2ae80dc9 | | Max. process+thread count | 415 (vs. ulimit of 5500) | | modules | C: hadoop-hdfs-project/hadoop-hdfs hadoop-hdfs-project/hadoop-hdfs-rbf U: hadoop-hdfs-project | | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-2096/3/console | | versions | git=2.17.1 maven=3.6.0 | | Powered by | Apache Yetus 0.12.0 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: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] sunchao commented on a change in pull request #2080: HDFS-15417. RBF: Get the datanode report from cache for federation WebHDFS operations
sunchao commented on a change in pull request #2080: URL: https://github.com/apache/hadoop/pull/2080#discussion_r449907165 ## File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRpc.java ## @@ -1777,6 +1777,43 @@ public void testgetGroupsForUser() throws IOException { assertArrayEquals(group, result); } + @Test + public void testGetCachedDatanodeReport() throws Exception { +final DatanodeInfo[] datanodeReport = +routerProtocol.getDatanodeReport(DatanodeReportType.ALL); + +// We should have 12 nodes in total +assertEquals(12, datanodeReport.length); + +// We should be caching this information +DatanodeInfo[] datanodeReport1 = +routerProtocol.getDatanodeReport(DatanodeReportType.ALL); +assertArrayEquals(datanodeReport1, datanodeReport); + +// Add one datanode + getCluster().getCluster().startDataNodes(getCluster().getCluster().getConfiguration(0), Review comment: I think we need to clear state after the test case so that it won't affect others like `testNamenodeMetrics`. Also long line. 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: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #2122: [HADOOP-17109] Replace Guava base64Url and base64 with Java8+ base64
hadoop-yetus commented on pull request #2122: URL: https://github.com/apache/hadoop/pull/2122#issuecomment-653885378 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 27m 4s | Docker mode activated. | ||| _ Prechecks _ | | +1 :green_heart: | dupname | 0m 1s | 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 14 new or modified test files. | ||| _ trunk Compile Tests _ | | +0 :ok: | mvndep | 0m 57s | Maven dependency ordering for branch | | +1 :green_heart: | mvninstall | 22m 36s | trunk passed | | +1 :green_heart: | compile | 21m 2s | trunk passed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04 | | +1 :green_heart: | compile | 17m 41s | trunk passed with JDK Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | +1 :green_heart: | checkstyle | 2m 55s | trunk passed | | +1 :green_heart: | mvnsite | 13m 21s | trunk passed | | +1 :green_heart: | shadedclient | 32m 4s | branch has no errors when building and testing our client artifacts. | | -1 :x: | javadoc | 0m 31s | hadoop-auth in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 37s | hadoop-common in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 30s | hadoop-kms in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 34s | hadoop-registry in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 44s | hadoop-hdfs-client in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 40s | hadoop-hdfs in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 32s | hadoop-hdfs-rbf in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 35s | hadoop-yarn-common in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 33s | hadoop-yarn-server-nodemanager in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 32s | hadoop-yarn-server-resourcemanager in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 32s | hadoop-yarn-client in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 35s | hadoop-aws in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 32s | hadoop-azure in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 28s | hadoop-cos in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | +1 :green_heart: | javadoc | 9m 51s | trunk passed with JDK Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | +0 :ok: | spotbugs | 0m 43s | Used deprecated FindBugs config; considering switching to SpotBugs. | | -1 :x: | findbugs | 0m 42s | hadoop-cloud-storage-project/hadoop-cos in trunk has 1 extant findbugs warnings. | ||| _ Patch Compile Tests _ | | +0 :ok: | mvndep | 0m 21s | Maven dependency ordering for patch | | +1 :green_heart: | mvninstall | 9m 46s | the patch passed | | +1 :green_heart: | compile | 20m 18s | the patch passed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04 | | -1 :x: | javac | 20m 18s | root-jdkUbuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04 with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04 generated 2 new + 1975 unchanged - 2 fixed = 1977 total (was 1977) | | +1 :green_heart: | compile | 17m 34s | the patch passed with JDK Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | -1 :x: | javac | 17m 34s | root-jdkPrivateBuild-1.8.0_252-8u252-b09-1~18.04-b09 with JDK Private Build-1.8.0_252-8u252-b09-1~18.04-b09 generated 2 new + 1868 unchanged - 2 fixed = 1870 total (was 1870) | | -0 :warning: | checkstyle | 2m 57s | root: The patch generated 20 new + 389 unchanged - 7 fixed = 409 total (was 396) | | +1 :green_heart: | mvnsite | 15m 45s | the patch passed | | +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. | | +1 :green_heart: | shadedclient | 17m 17s | patch has no errors when building and testing our client artifacts. | | -1 :x: | javadoc | 0m 31s | hadoop-auth in the patch failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | -1 :x: | javadoc | 0m 38s | hadoop-common in the patch failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. |
[GitHub] [hadoop] hadoop-yetus commented on pull request #2096: HDFS-15312. Apply umask when creating directory by WebHDFS
hadoop-yetus commented on pull request #2096: URL: https://github.com/apache/hadoop/pull/2096#issuecomment-653868788 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 25m 57s | Docker mode activated. | ||| _ Prechecks _ | | +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. | | +0 :ok: | jshint | 0m 0s | jshint was not available. | | +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. | ||| _ trunk Compile Tests _ | | +0 :ok: | mvndep | 1m 5s | Maven dependency ordering for branch | | +1 :green_heart: | mvninstall | 22m 35s | trunk passed | | +1 :green_heart: | shadedclient | 39m 13s | branch has no errors when building and testing our client artifacts. | ||| _ Patch Compile Tests _ | | +0 :ok: | mvndep | 0m 21s | Maven dependency ordering for patch | | +1 :green_heart: | mvninstall | 1m 39s | the patch passed | | -1 :x: | whitespace | 0m 0s | The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <>. Refer https://git-scm.com/docs/git-apply | | +1 :green_heart: | shadedclient | 15m 26s | patch has no errors when building and testing our client artifacts. | ||| _ Other Tests _ | | +1 :green_heart: | asflicense | 0m 28s | The patch does not generate ASF License warnings. | | | | 84m 50s | | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-2096/2/artifact/out/Dockerfile | | GITHUB PR | https://github.com/apache/hadoop/pull/2096 | | Optional Tests | dupname asflicense shadedclient jshint | | uname | Linux 0c745de0a6d9 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | personality/hadoop.sh | | git revision | trunk / 55a2ae80dc9 | | whitespace | https://builds.apache.org/job/hadoop-multibranch/job/PR-2096/2/artifact/out/whitespace-eol.txt | | Max. process+thread count | 308 (vs. ulimit of 5500) | | modules | C: hadoop-hdfs-project/hadoop-hdfs hadoop-hdfs-project/hadoop-hdfs-rbf U: hadoop-hdfs-project | | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-2096/2/console | | versions | git=2.17.1 maven=3.6.0 | | Powered by | Apache Yetus 0.12.0 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: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #2080: HDFS-15417. RBF: Get the datanode report from cache for federation WebHDFS operations
hadoop-yetus commented on pull request #2080: URL: https://github.com/apache/hadoop/pull/2080#issuecomment-653867087 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 0m 32s | 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 | 19m 8s | trunk passed | | +1 :green_heart: | compile | 0m 40s | trunk passed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04 | | +1 :green_heart: | compile | 0m 35s | trunk passed with JDK Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | +1 :green_heart: | checkstyle | 0m 25s | trunk passed | | +1 :green_heart: | mvnsite | 0m 38s | trunk passed | | +1 :green_heart: | shadedclient | 14m 47s | branch has no errors when building and testing our client artifacts. | | -1 :x: | javadoc | 0m 29s | hadoop-hdfs-rbf in trunk failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | +1 :green_heart: | javadoc | 0m 34s | trunk passed with JDK Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | +0 :ok: | spotbugs | 1m 8s | Used deprecated FindBugs config; considering switching to SpotBugs. | | +1 :green_heart: | findbugs | 1m 6s | trunk passed | ||| _ Patch Compile Tests _ | | +1 :green_heart: | mvninstall | 0m 31s | the patch passed | | +1 :green_heart: | compile | 0m 31s | the patch passed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04 | | +1 :green_heart: | javac | 0m 31s | the patch passed | | +1 :green_heart: | compile | 0m 27s | the patch passed with JDK Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | +1 :green_heart: | javac | 0m 27s | the patch passed | | -0 :warning: | checkstyle | 0m 18s | hadoop-hdfs-project/hadoop-hdfs-rbf: The patch generated 1 new + 16 unchanged - 0 fixed = 17 total (was 16) | | +1 :green_heart: | mvnsite | 0m 31s | the patch passed | | +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. | | +1 :green_heart: | shadedclient | 13m 44s | patch has no errors when building and testing our client artifacts. | | -1 :x: | javadoc | 0m 26s | hadoop-hdfs-rbf in the patch failed with JDK Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04. | | +1 :green_heart: | javadoc | 0m 30s | the patch passed with JDK Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | +1 :green_heart: | findbugs | 1m 12s | the patch passed | ||| _ Other Tests _ | | -1 :x: | unit | 7m 57s | hadoop-hdfs-rbf in the patch passed. | | +1 :green_heart: | asflicense | 0m 32s | The patch does not generate ASF License warnings. | | | | 68m 33s | | | Reason | Tests | |---:|:--| | Failed junit tests | hadoop.hdfs.server.federation.router.TestRouterRpc | | | hadoop.hdfs.server.federation.router.TestRouterRpcMultiDestination | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-2080/8/artifact/out/Dockerfile | | GITHUB PR | https://github.com/apache/hadoop/pull/2080 | | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle | | uname | Linux f63854f9d79c 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | personality/hadoop.sh | | git revision | trunk / 55a2ae80dc9 | | Default Java | Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | javadoc | https://builds.apache.org/job/hadoop-multibranch/job/PR-2080/8/artifact/out/branch-javadoc-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkUbuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04.txt | | checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-2080/8/artifact/out/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs-rbf.txt | | javadoc | https://builds.apache.org/job/hadoop-multibranch/job/PR-2080/8/artifact/out/patch-javadoc-hadoop-hdfs-project_hadoop-hdfs-rbf-jdkUbuntu-11.0.7+10-post-Ubuntu-2ubuntu218.04.txt | | unit | https://builds.apache.org/job/hadoop-multibranch/job/PR-2080/8/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt | | Test Results |
[GitHub] [hadoop] Hexiaoqiao commented on a change in pull request #2037: HDFS-14984. HDFS setQuota: Error message should be added for invalid …
Hexiaoqiao commented on a change in pull request #2037: URL: https://github.com/apache/hadoop/pull/2037#discussion_r449842359 ## File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSAdmin.java ## @@ -201,8 +201,19 @@ public void run(Path path) throws IOException { super(conf); CommandFormat c = new CommandFormat(2, Integer.MAX_VALUE); List parameters = c.parse(args, pos); - this.quota = - StringUtils.TraditionalBinaryPrefix.string2long(parameters.remove(0)); + String str = parameters.get(0).trim(); + try { +this.quota = StringUtils.TraditionalBinaryPrefix +.string2long(parameters.remove(0)); + } catch(NumberFormatException e){ +throw new IllegalArgumentException("\"" + str + +"\" is not a valid value for a quota."); + } + if (HdfsConstants.QUOTA_DONT_SET == this.quota) { +System.out.print("WARN: \"" + this.quota + Review comment: IMO, it should be using `System.err.print`. Another side, it is better to throw exception and no need to execute the following logic if meet invalid parameter. ## File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSAdmin.java ## @@ -323,6 +334,11 @@ public void run(Path path) throws IOException { } catch (NumberFormatException nfe) { throw new IllegalArgumentException("\"" + str + "\" is not a valid value for a quota."); } + if (HdfsConstants.QUOTA_DONT_SET == quota) { +System.out.print("WARN: \"" + this.quota + Review comment: the same as last comment. ## File path: hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSClient.java ## @@ -2553,16 +2553,17 @@ void setQuota(String src, long namespaceQuota, long storagespaceQuota) throws IOException { checkOpen(); // sanity check -if ((namespaceQuota <= 0 && - namespaceQuota != HdfsConstants.QUOTA_DONT_SET && - namespaceQuota != HdfsConstants.QUOTA_RESET) || -(storagespaceQuota < 0 && +if (namespaceQuota <= 0 && +namespaceQuota != HdfsConstants.QUOTA_DONT_SET && +namespaceQuota != HdfsConstants.QUOTA_RESET){ + throw new IllegalArgumentException("Invalid values for " + + "namespace quota : " + namespaceQuota); +} +if (storagespaceQuota < 0 && Review comment: this condition statement should be `if (storagesapceQuoat <= 0)`? 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: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org