[jira] [Commented] (YARN-10593) Fix incorrect string comparison in GpuDiscoverer
[ https://issues.apache.org/jira/browse/YARN-10593?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17282424#comment-17282424 ] Szilard Nemeth commented on YARN-10593: --- Thanks [~pbacsko] for working on this. Patch LGTM, committed to trunk. Thanks [~zhuqi] for the review. > Fix incorrect string comparison in GpuDiscoverer > > > Key: YARN-10593 > URL: https://issues.apache.org/jira/browse/YARN-10593 > Project: Hadoop YARN > Issue Type: Bug > Components: resourcemanager >Reporter: Peter Bacsko >Assignee: Peter Bacsko >Priority: Major > Fix For: 3.4.0 > > Attachments: YARN-10593-001.patch > > > The following comparison in {{GpuDiscoverer}} is invalid: > {noformat} > binaryPath = configuredBinaryFile; > // If path exists but file name is incorrect don't execute the file > String fileName = binaryPath.getName(); > if (DEFAULT_BINARY_NAME.equals(fileName)) { <--- inverse condition > needed > String msg = String.format("Please check the configuration value of" > +" %s. It should point to an %s binary.", > YarnConfiguration.NM_GPU_PATH_TO_EXEC, > DEFAULT_BINARY_NAME); > throwIfNecessary(new YarnException(msg), config); > LOG.warn(msg); > }{noformat} > Obviously it should be other way around - we should log a warning or throw an > exception if the file names *differ*, not when they're equal. > Consider adding a unit test for this. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-10593) Fix incorrect string comparison in GpuDiscoverer
[ https://issues.apache.org/jira/browse/YARN-10593?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17281049#comment-17281049 ] Hadoop QA commented on YARN-10593: -- | (/) *{color:green}+1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Logfile || Comment || | {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 46s{color} | {color:blue}{color} | {color:blue} Docker mode activated. {color} | || || || || {color:brown} Prechecks {color} || || | {color:green}+1{color} | {color:green} dupname {color} | {color:green} 0m 0s{color} | {color:green}{color} | {color:green} No case conflicting files found. {color} | | {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s{color} | {color:green}{color} | {color:green} The patch does not contain any @author tags. {color} | | {color:green}+1{color} | {color:green} {color} | {color:green} 0m 0s{color} | {color:green}test4tests{color} | {color:green} The patch appears to include 1 new or modified test files. {color} | || || || || {color:brown} trunk Compile Tests {color} || || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 19m 33s{color} | {color:green}{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m 28s{color} | {color:green}{color} | {color:green} trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m 21s{color} | {color:green}{color} | {color:green} trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 33s{color} | {color:green}{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 43s{color} | {color:green}{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 14m 10s{color} | {color:green}{color} | {color:green} branch has no errors when building and testing our client artifacts. {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 37s{color} | {color:green}{color} | {color:green} trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 35s{color} | {color:green}{color} | {color:green} trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 {color} | | {color:blue}0{color} | {color:blue} spotbugs {color} | {color:blue} 1m 20s{color} | {color:blue}{color} | {color:blue} Used deprecated FindBugs config; considering switching to SpotBugs. {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m 18s{color} | {color:green}{color} | {color:green} trunk passed {color} | || || || || {color:brown} Patch Compile Tests {color} || || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m 38s{color} | {color:green}{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m 18s{color} | {color:green}{color} | {color:green} the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 1m 18s{color} | {color:green}{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m 14s{color} | {color:green}{color} | {color:green} the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 1m 14s{color} | {color:green}{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 25s{color} | {color:green}{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 35s{color} | {color:green}{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m 0s{color} | {color:green}{color} | {color:green} The patch has no whitespace issues. {color} | | {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 12m 37s{color} | {color:green}{color} | {color:green} patch has no errors when building and testing our client artifacts. {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 34s{color} | {color:green}{color} | {color:green} the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 31s{color} | {color:green}{color} | {c
[jira] [Commented] (YARN-10593) Fix incorrect string comparison in GpuDiscoverer
[ https://issues.apache.org/jira/browse/YARN-10593?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17281042#comment-17281042 ] Qi Zhu commented on YARN-10593: --- Thanks [~pbacsko] for contribution. The patch LGTM. > Fix incorrect string comparison in GpuDiscoverer > > > Key: YARN-10593 > URL: https://issues.apache.org/jira/browse/YARN-10593 > Project: Hadoop YARN > Issue Type: Bug > Components: resourcemanager >Reporter: Peter Bacsko >Assignee: Peter Bacsko >Priority: Major > Attachments: YARN-10593-001.patch > > > The following comparison in {{GpuDiscoverer}} is invalid: > {noformat} > binaryPath = configuredBinaryFile; > // If path exists but file name is incorrect don't execute the file > String fileName = binaryPath.getName(); > if (DEFAULT_BINARY_NAME.equals(fileName)) { <--- inverse condition > needed > String msg = String.format("Please check the configuration value of" > +" %s. It should point to an %s binary.", > YarnConfiguration.NM_GPU_PATH_TO_EXEC, > DEFAULT_BINARY_NAME); > throwIfNecessary(new YarnException(msg), config); > LOG.warn(msg); > }{noformat} > Obviously it should be other way around - we should log a warning or throw an > exception if the file names *differ*, not when they're equal. > Consider adding a unit test for this. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-10593) Fix incorrect string comparison in GpuDiscoverer
[ https://issues.apache.org/jira/browse/YARN-10593?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17280948#comment-17280948 ] Peter Bacsko commented on YARN-10593: - [~zhuqi] thanks, but I already have a patch in progress, I will upload it soon. > Fix incorrect string comparison in GpuDiscoverer > > > Key: YARN-10593 > URL: https://issues.apache.org/jira/browse/YARN-10593 > Project: Hadoop YARN > Issue Type: Bug > Components: resourcemanager >Reporter: Peter Bacsko >Assignee: Peter Bacsko >Priority: Major > > The following comparison in {{GpuDiscoverer}} is invalid: > {noformat} > binaryPath = configuredBinaryFile; > // If path exists but file name is incorrect don't execute the file > String fileName = binaryPath.getName(); > if (DEFAULT_BINARY_NAME.equals(fileName)) { <--- inverse condition > needed > String msg = String.format("Please check the configuration value of" > +" %s. It should point to an %s binary.", > YarnConfiguration.NM_GPU_PATH_TO_EXEC, > DEFAULT_BINARY_NAME); > throwIfNecessary(new YarnException(msg), config); > LOG.warn(msg); > }{noformat} > Obviously it should be other way around - we should log a warning or throw an > exception if the file names *differ*, not when they're equal. > Consider adding a unit test for this. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-10593) Fix incorrect string comparison in GpuDiscoverer
[ https://issues.apache.org/jira/browse/YARN-10593?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17280350#comment-17280350 ] Qi Zhu commented on YARN-10593: --- [~pbacsko] May i take this? I can help to fix. > Fix incorrect string comparison in GpuDiscoverer > > > Key: YARN-10593 > URL: https://issues.apache.org/jira/browse/YARN-10593 > Project: Hadoop YARN > Issue Type: Bug > Components: resourcemanager >Reporter: Peter Bacsko >Assignee: Peter Bacsko >Priority: Major > > The following comparison in {{GpuDiscoverer}} is invalid: > {noformat} > binaryPath = configuredBinaryFile; > // If path exists but file name is incorrect don't execute the file > String fileName = binaryPath.getName(); > if (DEFAULT_BINARY_NAME.equals(fileName)) { <--- inverse condition > needed > String msg = String.format("Please check the configuration value of" > +" %s. It should point to an %s binary.", > YarnConfiguration.NM_GPU_PATH_TO_EXEC, > DEFAULT_BINARY_NAME); > throwIfNecessary(new YarnException(msg), config); > LOG.warn(msg); > }{noformat} > Obviously it should be other way around - we should log a warning or throw an > exception if the file names *differ*, not when they're equal. > Consider adding a unit test for this. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-10593) Fix incorrect string comparison in GpuDiscoverer
[ https://issues.apache.org/jira/browse/YARN-10593?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17271298#comment-17271298 ] Peter Bacsko commented on YARN-10593: - Unfortunately, Zoltan Siegl's comment has not been addressed: https://issues.apache.org/jira/browse/YARN-9217?focusedCommentId=16763445&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16763445 > Fix incorrect string comparison in GpuDiscoverer > > > Key: YARN-10593 > URL: https://issues.apache.org/jira/browse/YARN-10593 > Project: Hadoop YARN > Issue Type: Bug > Components: resourcemanager >Reporter: Peter Bacsko >Assignee: Peter Bacsko >Priority: Major > > The following comparison in {{GpuDiscoverer}} is invalid: > {noformat} > binaryPath = configuredBinaryFile; > // If path exists but file name is incorrect don't execute the file > String fileName = binaryPath.getName(); > if (DEFAULT_BINARY_NAME.equals(fileName)) { <--- inverse condition > needed > String msg = String.format("Please check the configuration value of" > +" %s. It should point to an %s binary.", > YarnConfiguration.NM_GPU_PATH_TO_EXEC, > DEFAULT_BINARY_NAME); > throwIfNecessary(new YarnException(msg), config); > LOG.warn(msg); > }{noformat} > Obviously it should be other way around - we should log a warning or throw an > exception if the file names *differ*, not when they're equal. > Consider adding a unit test for this. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org