[jira] [Commented] (YARN-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17141103#comment-17141103 ] Hudson commented on YARN-9460: -- SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #18370 (See [https://builds.apache.org/job/Hadoop-trunk-Commit/18370/]) YARN-9460. QueueACLsManager and ReservationsACLManager should not use (surendralilhore: rev b2facc84a1b48b9dcbe0816e120778d2100b320e) * (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ResourceManager.java * (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/security/CapacityReservationsACLsManager.java * (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/security/GenericQueueACLsManager.java * (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/security/QueueACLsManager.java * (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/reservation/AbstractReservationSystem.java * (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/security/package-info.java * (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/security/ReservationsACLsManager.java * (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/security/FairReservationsACLsManager.java * (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/security/CapacityQueueACLsManager.java * (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/security/FairQueueACLsManager.java * (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestClientRMTokens.java > QueueACLsManager and ReservationsACLManager should not use instanceof checks > > > Key: YARN-9460 > URL: https://issues.apache.org/jira/browse/YARN-9460 > Project: Hadoop YARN > Issue Type: Improvement > Components: resourcemanager >Reporter: Szilard Nemeth >Assignee: Bilwa S T >Priority: Major > Fix For: 3.4.0 > > Attachments: YARN-9460.001.patch, YARN-9460.002.patch, > YARN-9460.003.patch, YARN-9460.004.patch, YARN-9460.005.patch > > > QueueACLsManager and ReservationsACLManager should not use instanceof checks > for the scheduler type. > Rather, we should abstract this into two classes: Capacity and Fair variants > of these ACL classes. > QueueACLsManager and ReservationsACLManager could be abstract classes, but > the implementation is the decision of one who will work on this jira. -- 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-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17138552#comment-17138552 ] Hudáky Márton Gyula commented on YARN-9460: --- +1 (non-binding) > QueueACLsManager and ReservationsACLManager should not use instanceof checks > > > Key: YARN-9460 > URL: https://issues.apache.org/jira/browse/YARN-9460 > Project: Hadoop YARN > Issue Type: Improvement >Reporter: Szilard Nemeth >Assignee: Bilwa S T >Priority: Major > Attachments: YARN-9460.001.patch, YARN-9460.002.patch, > YARN-9460.003.patch, YARN-9460.004.patch, YARN-9460.005.patch > > > QueueACLsManager and ReservationsACLManager should not use instanceof checks > for the scheduler type. > Rather, we should abstract this into two classes: Capacity and Fair variants > of these ACL classes. > QueueACLsManager and ReservationsACLManager could be abstract classes, but > the implementation is the decision of one who will work on this jira. -- 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-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17135684#comment-17135684 ] Surendra Singh Lilhore commented on YARN-9460: -- +1, LGTM > QueueACLsManager and ReservationsACLManager should not use instanceof checks > > > Key: YARN-9460 > URL: https://issues.apache.org/jira/browse/YARN-9460 > Project: Hadoop YARN > Issue Type: Improvement >Reporter: Szilard Nemeth >Assignee: Bilwa S T >Priority: Major > Attachments: YARN-9460.001.patch, YARN-9460.002.patch, > YARN-9460.003.patch, YARN-9460.004.patch, YARN-9460.005.patch > > > QueueACLsManager and ReservationsACLManager should not use instanceof checks > for the scheduler type. > Rather, we should abstract this into two classes: Capacity and Fair variants > of these ACL classes. > QueueACLsManager and ReservationsACLManager could be abstract classes, but > the implementation is the decision of one who will work on this jira. -- 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-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17133381#comment-17133381 ] Hadoop QA commented on YARN-9460: - | (/) *{color:green}+1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 1m 20s{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} No case conflicting files found. {color} | | {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s{color} | {color:green} The patch does not contain any @author tags. {color} | | {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m 0s{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} 21m 7s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 46s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 37s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 49s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 18m 40s{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 40s{color} | {color:green} trunk passed {color} | | {color:blue}0{color} | {color:blue} spotbugs {color} | {color:blue} 2m 2s{color} | {color:blue} Used deprecated FindBugs config; considering switching to SpotBugs. {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 2m 0s{color} | {color:green} trunk passed {color} | || || || || {color:brown} Patch Compile Tests {color} || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m 51s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 51s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 51s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 32s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 45s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m 0s{color} | {color:green} The patch has no whitespace issues. {color} | | {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 15m 38s{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 27s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m 44s{color} | {color:green} the patch passed {color} | || || || || {color:brown} Other Tests {color} || | {color:green}+1{color} | {color:green} unit {color} | {color:green} 91m 52s{color} | {color:green} hadoop-yarn-server-resourcemanager in the patch passed. {color} | | {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m 28s{color} | {color:green} The patch does not generate ASF License warnings. {color} | | {color:black}{color} | {color:black} {color} | {color:black}159m 7s{color} | {color:black} {color} | \\ \\ || Subsystem || Report/Notes || | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://builds.apache.org/job/PreCommit-YARN-Build/26150/artifact/out/Dockerfile | | JIRA Issue | YARN-9460 | | JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/13005496/YARN-9460.005.patch | | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle | | uname | Linux d11010bca91d 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 / 93b121a9717 | | Default Java | Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | Test Results | https://builds.apache.org/job/PreCommit-YARN-Build/26150/testReport/ | | Max. process+thread count | 838 (vs. ulimit of 5500) | | modules | C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-ya
[jira] [Commented] (YARN-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17133345#comment-17133345 ] Peter Bacsko commented on YARN-9460: +1 (non-binding) pending Jenkins > QueueACLsManager and ReservationsACLManager should not use instanceof checks > > > Key: YARN-9460 > URL: https://issues.apache.org/jira/browse/YARN-9460 > Project: Hadoop YARN > Issue Type: Improvement >Reporter: Szilard Nemeth >Assignee: Bilwa S T >Priority: Major > Attachments: YARN-9460.001.patch, YARN-9460.002.patch, > YARN-9460.003.patch, YARN-9460.004.patch, YARN-9460.005.patch > > > QueueACLsManager and ReservationsACLManager should not use instanceof checks > for the scheduler type. > Rather, we should abstract this into two classes: Capacity and Fair variants > of these ACL classes. > QueueACLsManager and ReservationsACLManager could be abstract classes, but > the implementation is the decision of one who will work on this jira. -- 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-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17133277#comment-17133277 ] Bilwa S T commented on YARN-9460: - added @SuppressWarnings("checkstyle:visibilitymodifier") to avoid checkstyle issues > QueueACLsManager and ReservationsACLManager should not use instanceof checks > > > Key: YARN-9460 > URL: https://issues.apache.org/jira/browse/YARN-9460 > Project: Hadoop YARN > Issue Type: Improvement >Reporter: Szilard Nemeth >Assignee: Bilwa S T >Priority: Major > Attachments: YARN-9460.001.patch, YARN-9460.002.patch, > YARN-9460.003.patch, YARN-9460.004.patch, YARN-9460.005.patch > > > QueueACLsManager and ReservationsACLManager should not use instanceof checks > for the scheduler type. > Rather, we should abstract this into two classes: Capacity and Fair variants > of these ACL classes. > QueueACLsManager and ReservationsACLManager could be abstract classes, but > the implementation is the decision of one who will work on this jira. -- 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-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17133081#comment-17133081 ] Peter Bacsko commented on YARN-9460: Thanks [~BilwaST] I have no further comments. > QueueACLsManager and ReservationsACLManager should not use instanceof checks > > > Key: YARN-9460 > URL: https://issues.apache.org/jira/browse/YARN-9460 > Project: Hadoop YARN > Issue Type: Improvement >Reporter: Szilard Nemeth >Assignee: Bilwa S T >Priority: Major > Attachments: YARN-9460.001.patch, YARN-9460.002.patch, > YARN-9460.003.patch, YARN-9460.004.patch > > > QueueACLsManager and ReservationsACLManager should not use instanceof checks > for the scheduler type. > Rather, we should abstract this into two classes: Capacity and Fair variants > of these ACL classes. > QueueACLsManager and ReservationsACLManager could be abstract classes, but > the implementation is the decision of one who will work on this jira. -- 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-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17130456#comment-17130456 ] Bilwa S T commented on YARN-9460: - Hi [~pbacsko] # I will update {{@SuppressWarnings("checkstyle:visibilitymodifier")}} to the class. # It will not cause NPE as there is null check in ClientRMService#checkReservationACLs. if ReservationsACLsManager is null then it returns callerUGI.getShortUserName(). > QueueACLsManager and ReservationsACLManager should not use instanceof checks > > > Key: YARN-9460 > URL: https://issues.apache.org/jira/browse/YARN-9460 > Project: Hadoop YARN > Issue Type: Improvement >Reporter: Szilard Nemeth >Assignee: Bilwa S T >Priority: Major > Attachments: YARN-9460.001.patch, YARN-9460.002.patch, > YARN-9460.003.patch, YARN-9460.004.patch > > > QueueACLsManager and ReservationsACLManager should not use instanceof checks > for the scheduler type. > Rather, we should abstract this into two classes: Capacity and Fair variants > of these ACL classes. > QueueACLsManager and ReservationsACLManager could be abstract classes, but > the implementation is the decision of one who will work on this jira. -- 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-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17130447#comment-17130447 ] Peter Bacsko commented on YARN-9460: Another comment: {noformat} if (scheduler instanceof CapacityScheduler) { reservationsACLsManager = new CapacityReservationsACLsManager(scheduler, conf); } else if (scheduler instanceof FairScheduler) { reservationsACLsManager = new FairReservationsACLsManager(scheduler, conf); } {noformat} This is very similar to what I mentioned above. If neither CS nor FS is set, then this can cause an NPE. We need a ReservationsACLsManager which does nothing in a final else branch, eg. {{PassThroughReservationsACLsManager}} which always return true for {{checkAccess()}}. > QueueACLsManager and ReservationsACLManager should not use instanceof checks > > > Key: YARN-9460 > URL: https://issues.apache.org/jira/browse/YARN-9460 > Project: Hadoop YARN > Issue Type: Improvement >Reporter: Szilard Nemeth >Assignee: Bilwa S T >Priority: Major > Attachments: YARN-9460.001.patch, YARN-9460.002.patch, > YARN-9460.003.patch, YARN-9460.004.patch > > > QueueACLsManager and ReservationsACLManager should not use instanceof checks > for the scheduler type. > Rather, we should abstract this into two classes: Capacity and Fair variants > of these ACL classes. > QueueACLsManager and ReservationsACLManager could be abstract classes, but > the implementation is the decision of one who will work on this jira. -- 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-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17130411#comment-17130411 ] Peter Bacsko commented on YARN-9460: Thanks for the new patch [~BilwaST]. There are some checkstyle "VisibilityModifier" complaints. Please validate if those are legitimate or not (I can see that they're package private). If they're accessed in the descendant classes, make them protected. If package private is legit, then add {{@SuppressWarnings("checkstyle:visibilitymodifier")}} to the class. > QueueACLsManager and ReservationsACLManager should not use instanceof checks > > > Key: YARN-9460 > URL: https://issues.apache.org/jira/browse/YARN-9460 > Project: Hadoop YARN > Issue Type: Improvement >Reporter: Szilard Nemeth >Assignee: Bilwa S T >Priority: Major > Attachments: YARN-9460.001.patch, YARN-9460.002.patch, > YARN-9460.003.patch, YARN-9460.004.patch > > > QueueACLsManager and ReservationsACLManager should not use instanceof checks > for the scheduler type. > Rather, we should abstract this into two classes: Capacity and Fair variants > of these ACL classes. > QueueACLsManager and ReservationsACLManager could be abstract classes, but > the implementation is the decision of one who will work on this jira. -- 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-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17129680#comment-17129680 ] Hadoop QA commented on YARN-9460: - | (/) *{color:green}+1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 1m 23s{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} No case conflicting files found. {color} | | {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s{color} | {color:green} The patch does not contain any @author tags. {color} | | {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m 0s{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} 21m 45s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 47s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 35s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 50s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 16m 38s{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 31s{color} | {color:green} trunk passed {color} | | {color:blue}0{color} | {color:blue} spotbugs {color} | {color:blue} 1m 41s{color} | {color:blue} Used deprecated FindBugs config; considering switching to SpotBugs. {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m 40s{color} | {color:green} trunk passed {color} | || || || || {color:brown} Patch Compile Tests {color} || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m 48s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 42s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 42s{color} | {color:green} the patch passed {color} | | {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange} 0m 31s{color} | {color:orange} hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 4 new + 81 unchanged - 0 fixed = 85 total (was 81) {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 43s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m 0s{color} | {color:green} The patch has no whitespace issues. {color} | | {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 15m 13s{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 29s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m 46s{color} | {color:green} the patch passed {color} | || || || || {color:brown} Other Tests {color} || | {color:green}+1{color} | {color:green} unit {color} | {color:green} 91m 14s{color} | {color:green} hadoop-yarn-server-resourcemanager in the patch passed. {color} | | {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m 28s{color} | {color:green} The patch does not generate ASF License warnings. {color} | | {color:black}{color} | {color:black} {color} | {color:black}155m 59s{color} | {color:black} {color} | \\ \\ || Subsystem || Report/Notes || | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://builds.apache.org/job/PreCommit-YARN-Build/26141/artifact/out/Dockerfile | | JIRA Issue | YARN-9460 | | JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/13005257/YARN-9460.004.patch | | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle | | uname | Linux 775a6fa5c65b 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 / ac5d899d40d | | Default Java | Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | checkstyle | https://builds.apache.org/job/PreCommit-YARN-Build/26141/artifact/out/diff-checkstyle-hadoop-yarn-p
[jira] [Commented] (YARN-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17127016#comment-17127016 ] Hadoop QA commented on YARN-9460: - | (/) *{color:green}+1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 1m 25s{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} No case conflicting files found. {color} | | {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s{color} | {color:green} The patch does not contain any @author tags. {color} | | {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m 0s{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} 25m 22s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 49s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 37s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 53s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 16m 51s{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 31s{color} | {color:green} trunk passed {color} | | {color:blue}0{color} | {color:blue} spotbugs {color} | {color:blue} 1m 42s{color} | {color:blue} Used deprecated FindBugs config; considering switching to SpotBugs. {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m 39s{color} | {color:green} trunk passed {color} | || || || || {color:brown} Patch Compile Tests {color} || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m 48s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 42s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 42s{color} | {color:green} the patch passed {color} | | {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange} 0m 32s{color} | {color:orange} hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 8 new + 81 unchanged - 0 fixed = 89 total (was 81) {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 44s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m 0s{color} | {color:green} The patch has no whitespace issues. {color} | | {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 15m 19s{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 29s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m 48s{color} | {color:green} the patch passed {color} | || || || || {color:brown} Other Tests {color} || | {color:green}+1{color} | {color:green} unit {color} | {color:green} 93m 5s{color} | {color:green} hadoop-yarn-server-resourcemanager in the patch passed. {color} | | {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m 29s{color} | {color:green} The patch does not generate ASF License warnings. {color} | | {color:black}{color} | {color:black} {color} | {color:black}161m 58s{color} | {color:black} {color} | \\ \\ || Subsystem || Report/Notes || | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://builds.apache.org/job/PreCommit-YARN-Build/26122/artifact/out/Dockerfile | | JIRA Issue | YARN-9460 | | JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/13004951/YARN-9460.003.patch | | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle | | uname | Linux 2300370d24fb 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 / 23261237054 | | Default Java | Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | checkstyle | https://builds.apache.org/job/PreCommit-YARN-Build/26122/artifact/out/diff-checkstyle-hadoop-yarn-p
[jira] [Commented] (YARN-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17126919#comment-17126919 ] Bilwa S T commented on YARN-9460: - Hi [~pbacsko] Thanks for review. Yes you are correct. behaviour wont change in this case. I had this in mind but was not sure. I have uploaded patch. Please take a look at it > QueueACLsManager and ReservationsACLManager should not use instanceof checks > > > Key: YARN-9460 > URL: https://issues.apache.org/jira/browse/YARN-9460 > Project: Hadoop YARN > Issue Type: Improvement >Reporter: Szilard Nemeth >Assignee: Bilwa S T >Priority: Major > Attachments: YARN-9460.001.patch, YARN-9460.002.patch, > YARN-9460.003.patch > > > QueueACLsManager and ReservationsACLManager should not use instanceof checks > for the scheduler type. > Rather, we should abstract this into two classes: Capacity and Fair variants > of these ACL classes. > QueueACLsManager and ReservationsACLManager could be abstract classes, but > the implementation is the decision of one who will work on this jira. -- 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-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17126673#comment-17126673 ] Peter Bacsko commented on YARN-9460: [~BilwaST] I have a comment. Right now if the scheduler is not FS/CS, we just perform a {{scheduler.checkAccess()}}. Your patch changes this behaviour because in case of an unknown scheduler type, it returns null as {{QueueACLsManager}} implementation. I think it would be better to return sth like a {{GenericACLsManager}} which also does the same thing. I suggest renaming {{FifoQueueACLsManager}} to {{GenericACLsManager}} and use this if not FS/CS is configured. > QueueACLsManager and ReservationsACLManager should not use instanceof checks > > > Key: YARN-9460 > URL: https://issues.apache.org/jira/browse/YARN-9460 > Project: Hadoop YARN > Issue Type: Improvement >Reporter: Szilard Nemeth >Assignee: Bilwa S T >Priority: Major > Attachments: YARN-9460.001.patch, YARN-9460.002.patch > > > QueueACLsManager and ReservationsACLManager should not use instanceof checks > for the scheduler type. > Rather, we should abstract this into two classes: Capacity and Fair variants > of these ACL classes. > QueueACLsManager and ReservationsACLManager could be abstract classes, but > the implementation is the decision of one who will work on this jira. -- 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-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17126465#comment-17126465 ] Bilwa S T commented on YARN-9460: - cc [~inigoiri] > QueueACLsManager and ReservationsACLManager should not use instanceof checks > > > Key: YARN-9460 > URL: https://issues.apache.org/jira/browse/YARN-9460 > Project: Hadoop YARN > Issue Type: Improvement >Reporter: Szilard Nemeth >Assignee: Bilwa S T >Priority: Major > Attachments: YARN-9460.001.patch, YARN-9460.002.patch > > > QueueACLsManager and ReservationsACLManager should not use instanceof checks > for the scheduler type. > Rather, we should abstract this into two classes: Capacity and Fair variants > of these ACL classes. > QueueACLsManager and ReservationsACLManager could be abstract classes, but > the implementation is the decision of one who will work on this jira. -- 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-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17118868#comment-17118868 ] Bilwa S T commented on YARN-9460: - [~snemeth] Please review when you have free time > QueueACLsManager and ReservationsACLManager should not use instanceof checks > > > Key: YARN-9460 > URL: https://issues.apache.org/jira/browse/YARN-9460 > Project: Hadoop YARN > Issue Type: Improvement >Reporter: Szilard Nemeth >Assignee: Bilwa S T >Priority: Major > Attachments: YARN-9460.001.patch, YARN-9460.002.patch > > > QueueACLsManager and ReservationsACLManager should not use instanceof checks > for the scheduler type. > Rather, we should abstract this into two classes: Capacity and Fair variants > of these ACL classes. > QueueACLsManager and ReservationsACLManager could be abstract classes, but > the implementation is the decision of one who will work on this jira. -- 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-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17107552#comment-17107552 ] Bilwa S T commented on YARN-9460: - Hi [~snemeth] can you please review this? > QueueACLsManager and ReservationsACLManager should not use instanceof checks > > > Key: YARN-9460 > URL: https://issues.apache.org/jira/browse/YARN-9460 > Project: Hadoop YARN > Issue Type: Improvement >Reporter: Szilard Nemeth >Assignee: Bilwa S T >Priority: Major > Attachments: YARN-9460.001.patch, YARN-9460.002.patch > > > QueueACLsManager and ReservationsACLManager should not use instanceof checks > for the scheduler type. > Rather, we should abstract this into two classes: Capacity and Fair variants > of these ACL classes. > QueueACLsManager and ReservationsACLManager could be abstract classes, but > the implementation is the decision of one who will work on this jira. -- 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-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17102384#comment-17102384 ] Hadoop QA commented on YARN-9460: - | (/) *{color:green}+1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 2m 11s{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} No case conflicting files found. {color} | | {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s{color} | {color:green} The patch does not contain any @author tags. {color} | | {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m 0s{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} 27m 17s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m 3s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 51s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 1m 8s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 20m 25s{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 32s{color} | {color:green} trunk passed {color} | | {color:blue}0{color} | {color:blue} spotbugs {color} | {color:blue} 1m 58s{color} | {color:blue} Used deprecated FindBugs config; considering switching to SpotBugs. {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m 55s{color} | {color:green} trunk passed {color} | || || || || {color:brown} Patch Compile Tests {color} || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m 55s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 47s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 47s{color} | {color:green} the patch passed {color} | | {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange} 0m 34s{color} | {color:orange} hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 6 new + 108 unchanged - 0 fixed = 114 total (was 108) {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 50s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m 0s{color} | {color:green} The patch has no whitespace issues. {color} | | {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 17m 24s{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 37s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 2m 2s{color} | {color:green} the patch passed {color} | || || || || {color:brown} Other Tests {color} || | {color:green}+1{color} | {color:green} unit {color} | {color:green} 91m 56s{color} | {color:green} hadoop-yarn-server-resourcemanager in the patch passed. {color} | | {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m 28s{color} | {color:green} The patch does not generate ASF License warnings. {color} | | {color:black}{color} | {color:black} {color} | {color:black}170m 22s{color} | {color:black} {color} | \\ \\ || Subsystem || Report/Notes || | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://builds.apache.org/job/PreCommit-YARN-Build/26005/artifact/out/Dockerfile | | JIRA Issue | YARN-9460 | | JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/13002368/YARN-9460.002.patch | | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle | | uname | Linux d1919662cd60 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 / e9e1ead089c | | Default Java | Private Build-1.8.0_252-8u252-b09-1~18.04-b09 | | checkstyle | https://builds.apache.org/job/PreCommit-YARN-Build/26005/artifact/out/diff-checkstyle-hadoop-yarn-
[jira] [Commented] (YARN-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17102016#comment-17102016 ] Hadoop QA commented on YARN-9460: - | (x) *{color:red}-1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 1m 31s{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} No case conflicting files found. {color} | | {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s{color} | {color:green} The patch does not contain any @author tags. {color} | | {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m 0s{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} 24m 56s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 49s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 41s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 52s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 17m 47s{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 34s{color} | {color:green} trunk passed {color} | | {color:blue}0{color} | {color:blue} spotbugs {color} | {color:blue} 2m 1s{color} | {color:blue} Used deprecated FindBugs config; considering switching to SpotBugs. {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 2m 0s{color} | {color:green} trunk passed {color} | || || || || {color:brown} Patch Compile Tests {color} || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m 51s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 45s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 45s{color} | {color:green} the patch passed {color} | | {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange} 0m 34s{color} | {color:orange} hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 14 new + 108 unchanged - 0 fixed = 122 total (was 108) {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 48s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m 0s{color} | {color:green} The patch has no whitespace issues. {color} | | {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 15m 48s{color} | {color:green} patch has no errors when building and testing our client artifacts. {color} | | {color:red}-1{color} | {color:red} javadoc {color} | {color:red} 0m 32s{color} | {color:red} hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 5 new + 69 unchanged - 0 fixed = 74 total (was 69) {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 2m 4s{color} | {color:green} the patch passed {color} | || || || || {color:brown} Other Tests {color} || | {color:red}-1{color} | {color:red} unit {color} | {color:red} 75m 30s{color} | {color:red} hadoop-yarn-server-resourcemanager in the patch passed. {color} | | {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m 46s{color} | {color:green} The patch does not generate ASF License warnings. {color} | | {color:black}{color} | {color:black} {color} | {color:black}146m 36s{color} | {color:black} {color} | \\ \\ || Reason || Tests || | Failed junit tests | hadoop.yarn.server.resourcemanager.webapp.fairscheduler.TestRMWebServicesFairScheduler | | | hadoop.yarn.server.resourcemanager.webapp.fairscheduler.TestRMWebServicesFairSchedulerCustomResourceTypes | | | hadoop.yarn.server.resourcemanager.webapp.TestRMWebServicesReservation | \\ \\ || Subsystem || Report/Notes || | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://builds.apache.org/job/PreCommit-YARN-Build/26003/artifact/out/Dockerfile | | JIRA Issue | YARN-9460 | | JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/13002329/YARN-9460.001.patch | | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvns
[jira] [Commented] (YARN-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17101920#comment-17101920 ] Bilwa S T commented on YARN-9460: - Hi [~snemeth] Thanks for clarification. I didn't add SchedulerType enum as Fifo scheduler doesn't have implemenation for ReservationsACLManager . So i initialized based on instanceof checks. > QueueACLsManager and ReservationsACLManager should not use instanceof checks > > > Key: YARN-9460 > URL: https://issues.apache.org/jira/browse/YARN-9460 > Project: Hadoop YARN > Issue Type: Improvement >Reporter: Szilard Nemeth >Assignee: Bilwa S T >Priority: Major > Attachments: YARN-9460.001.patch > > > QueueACLsManager and ReservationsACLManager should not use instanceof checks > for the scheduler type. > Rather, we should abstract this into two classes: Capacity and Fair variants > of these ACL classes. > QueueACLsManager and ReservationsACLManager could be abstract classes, but > the implementation is the decision of one who will work on this jira. -- 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-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17099779#comment-17099779 ] Szilard Nemeth commented on YARN-9460: -- Hi [~BilwaST], Oh, I see what is your question now. I think we can do initialize the appropriate type of queueAclsManager based on instanceof checks. Perhaps, a better way to do it is to introduce a SchedulerType enum with 3 values: fair, capacity, fifo (if we don't have a similar thing in the codebase currently). Anyway, this could complicate a code a bit more so I'll let you decide what is the better way. > QueueACLsManager and ReservationsACLManager should not use instanceof checks > > > Key: YARN-9460 > URL: https://issues.apache.org/jira/browse/YARN-9460 > Project: Hadoop YARN > Issue Type: Improvement >Reporter: Szilard Nemeth >Assignee: Bilwa S T >Priority: Major > > QueueACLsManager and ReservationsACLManager should not use instanceof checks > for the scheduler type. > Rather, we should abstract this into two classes: Capacity and Fair variants > of these ACL classes. > QueueACLsManager and ReservationsACLManager could be abstract classes, but > the implementation is the decision of one who will work on this jira. -- 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-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17099773#comment-17099773 ] Bilwa S T commented on YARN-9460: - Hi [~snemeth] Thanks for the clarification. This is clear from Jira description itself. My doubt was about initializing queueAclsManager. Do we initialize based on scheduler type ? like below {code:java} public static String getDefaultReservationSystem( ResourceScheduler scheduler) { if (scheduler instanceof CapacityScheduler) { return CapacityReservationSystem.class.getName(); } else if (scheduler instanceof FairScheduler) { return FairReservationSystem.class.getName(); } return null; } {code} > QueueACLsManager and ReservationsACLManager should not use instanceof checks > > > Key: YARN-9460 > URL: https://issues.apache.org/jira/browse/YARN-9460 > Project: Hadoop YARN > Issue Type: Improvement >Reporter: Szilard Nemeth >Assignee: Bilwa S T >Priority: Major > > QueueACLsManager and ReservationsACLManager should not use instanceof checks > for the scheduler type. > Rather, we should abstract this into two classes: Capacity and Fair variants > of these ACL classes. > QueueACLsManager and ReservationsACLManager could be abstract classes, but > the implementation is the decision of one who will work on this jira. -- 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-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17099760#comment-17099760 ] Szilard Nemeth commented on YARN-9460: -- Hi [~BilwaST], One example of using instanceof is here: https://github.com/apache/hadoop/blob/9c8236d04dfc3d4cefe7a00b63625f60ee232cfe/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/security/QueueACLsManager.java#L61 Sometimes, instanceof checks might suggest a bad code design. My idea is to create 2 versions from QueueACLsManager: One for FairScheduler and one for CapacityScheduler. This way, checkAccess methods can be implemented differently so the instanceof checks could be replaced properly with Object oriented design. Hope this makes sense. If not, please feel free to ask follow up questions. Thanks. > QueueACLsManager and ReservationsACLManager should not use instanceof checks > > > Key: YARN-9460 > URL: https://issues.apache.org/jira/browse/YARN-9460 > Project: Hadoop YARN > Issue Type: Improvement >Reporter: Szilard Nemeth >Assignee: Bilwa S T >Priority: Major > > QueueACLsManager and ReservationsACLManager should not use instanceof checks > for the scheduler type. > Rather, we should abstract this into two classes: Capacity and Fair variants > of these ACL classes. > QueueACLsManager and ReservationsACLManager could be abstract classes, but > the implementation is the decision of one who will work on this jira. -- 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-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17094724#comment-17094724 ] Bilwa S T commented on YARN-9460: - Hi [~snemeth] I had one small doubt. So we make these as configurable or decide based on scheduler type?? > QueueACLsManager and ReservationsACLManager should not use instanceof checks > > > Key: YARN-9460 > URL: https://issues.apache.org/jira/browse/YARN-9460 > Project: Hadoop YARN > Issue Type: Improvement >Reporter: Szilard Nemeth >Assignee: Bilwa S T >Priority: Major > > QueueACLsManager and ReservationsACLManager should not use instanceof checks > for the scheduler type. > Rather, we should abstract this into two classes: Capacity and Fair variants > of these ACL classes. > QueueACLsManager and ReservationsACLManager could be abstract classes, but > the implementation is the decision of one who will work on this jira. -- 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-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16812916#comment-16812916 ] Bibin A Chundatt commented on YARN-9460: Porbably in YARN-9459 then.. Probably best to include in YarnAuthorizationProvider > QueueACLsManager and ReservationsACLManager should not use instanceof checks > > > Key: YARN-9460 > URL: https://issues.apache.org/jira/browse/YARN-9460 > Project: Hadoop YARN > Issue Type: Improvement >Reporter: Szilard Nemeth >Priority: Major > > QueueACLsManager and ReservationsACLManager should not use instanceof checks > for the scheduler type. > Rather, we should abstract this into two classes: Capacity and Fair variants > of these ACL classes. > QueueACLsManager and ReservationsACLManager could be abstract classes, but > the implementation is the decision of one who will work on this jira. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16812455#comment-16812455 ] Szilard Nemeth commented on YARN-9460: -- Hi [~bibinchundatt]! It's a good idea but ClientRMService does not belong to these ACL manager classes in any way, so I would file a separate jira if you want to eliminate instanceof from that class. > QueueACLsManager and ReservationsACLManager should not use instanceof checks > > > Key: YARN-9460 > URL: https://issues.apache.org/jira/browse/YARN-9460 > Project: Hadoop YARN > Issue Type: Improvement >Reporter: Szilard Nemeth >Priority: Major > > QueueACLsManager and ReservationsACLManager should not use instanceof checks > for the scheduler type. > Rather, we should abstract this into two classes: Capacity and Fair variants > of these ACL classes. > QueueACLsManager and ReservationsACLManager could be abstract classes, but > the implementation is the decision of one who will work on this jira. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-9460) QueueACLsManager and ReservationsACLManager should not use instanceof checks
[ https://issues.apache.org/jira/browse/YARN-9460?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16812437#comment-16812437 ] Bibin A Chundatt commented on YARN-9460: Thank you [~snemeth] for raising Consider ClientRMService#createAndPopulateNewRMApp scheduler instance of check also as part of this jira > QueueACLsManager and ReservationsACLManager should not use instanceof checks > > > Key: YARN-9460 > URL: https://issues.apache.org/jira/browse/YARN-9460 > Project: Hadoop YARN > Issue Type: Improvement >Reporter: Szilard Nemeth >Priority: Major > > QueueACLsManager and ReservationsACLManager should not use instanceof checks > for the scheduler type. > Rather, we should abstract this into two classes: Capacity and Fair variants > of these ACL classes. > QueueACLsManager and ReservationsACLManager could be abstract classes, but > the implementation is the decision of one who will work on this jira. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org