[jira] [Comment Edited] (YARN-9996) Code cleanup in QueueAdminConfigurationMutationACLPolicy
[ https://issues.apache.org/jira/browse/YARN-9996?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17081166#comment-17081166 ] Siddharth Ahuja edited comment on YARN-9996 at 4/11/20, 9:06 AM: - # Instead of manually extracting out the last child of the queue in the for-loop and then doing the same activity in the while-loop for the trimmed queue, my code makes this neater (and simpler) by pulling in the logic outside of the while-loop into the while-loop itself. This code optimization makes a saving of a call each to lastIndexOf() and substring(). # Further, I have made private methods for calls containing lastIndexOf() and substring() for the different scenarios to provide better context of what these calls are about. This abstracts away the need for the person reviewing the code to have to understand the internal logic of how and what needs to be done to extract out the queue names. # Added logging wherever necessary to provide more context. # No new JUnits added as the changes do not introduce/modify existing use-cases, they only optimize them i.e. changes are cosmetic at the end of the day. Class - org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.conf.TestConfigurationMutationACLPolicies already contains test methods relevant for QueueAdminConfigurationMutationACLPolicy. Re-ran the existing test methods to ensure changes are good. was (Author: sahuja): # Instead of manually extracting out the last child of the queue in the for-loop and then doing the same activity in the while-loop for the trimmed queue, my code makes this neater my pulling in the logic outside of the while-loop into the while-loop itself. This code optimization makes a saving of a call each to lastIndexOf() and substring(). # Further, I have made private methods for calls containing lastIndexOf() and substring() for the different scenarios to provide better context of what these calls are about. This abstracts away the need for the person reviewing the code to have to understand the internal logic of how and what needs to be done to extract out the queue names. # Added logging wherever necessary to provide more context. # No new JUnits added as the changes do not introduce/modify existing use-cases, they only optimize them i.e. changes are cosmetic at the end of the day. Class - org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.conf.TestConfigurationMutationACLPolicies already contains test methods relevant for QueueAdminConfigurationMutationACLPolicy. Re-ran the existing test methods to ensure changes are good. > Code cleanup in QueueAdminConfigurationMutationACLPolicy > > > Key: YARN-9996 > URL: https://issues.apache.org/jira/browse/YARN-9996 > Project: Hadoop YARN > Issue Type: Improvement >Reporter: Szilard Nemeth >Assignee: Siddharth Ahuja >Priority: Major > Attachments: YARN-9996.001.patch > > > Method 'isMutationAllowed' contains many uses of substring and lastIndexOf. > These could be extracted and simplified. > Also, some logging could be added as well. -- 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] [Comment Edited] (YARN-9996) Code cleanup in QueueAdminConfigurationMutationACLPolicy
[
https://issues.apache.org/jira/browse/YARN-9996?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17081178#comment-17081178
]
Siddharth Ahuja edited comment on YARN-9996 at 4/11/20, 9:06 AM:
-
Further details:
The overall aim of the code (as I understand) is to get the last child in the
queue hierarchy that has queue information available to check for admin user
access. Until this queue information is available, keep going up in the queue
hierarchy one child at a time.
+*Before* (un-important bits extracted away):+
{code}
for each queue
{
if the queue has a child, get the last child.
Try getting the queue info for this child.
while we do not have queue info
{
get the queue minus its last child (move up in the hierarchy)
if the trimmed queue has a child, get the last child.
Try getting the queue info for this child.
}
...
}
{code}
+*After* (un-important bits extracted away):+
{code}
for each queue
{
while we do not have queue info
{
if the queue has a child, get the last child.
Try getting the queue info for this child.
if the queue has a child, trim the queue and get the queue minus its
last child (move up in the hierarchy).
}
...
}
{code}
was (Author: sahuja):
Further details:
The overall aim of the code (as understood) is to get the last child in the
queue hierarchy that has queue information available to check for admin user
access. Until this queue information is available, keep going up in the queue
hierarchy one child at a time.
+*Before* (un-important bits extracted away):+
{code}
for each queue
{
if the queue has a child, get the last child.
Try getting the queue info for this child.
while we do not have queue info
{
get the queue minus its last child (move up in the hierarchy)
if the trimmed queue has a child, get the last child.
Try getting the queue info for this child.
}
...
}
{code}
+*After* (un-important bits extracted away):+
{code}
for each queue
{
while we do not have queue info
{
if the queue has a child, get the last child.
Try getting the queue info for this child.
if the queue has a child, trim the queue and get the queue minus its
last child (move up in the hierarchy).
}
...
}
{code}
> Code cleanup in QueueAdminConfigurationMutationACLPolicy
>
>
> Key: YARN-9996
> URL: https://issues.apache.org/jira/browse/YARN-9996
> Project: Hadoop YARN
> Issue Type: Improvement
>Reporter: Szilard Nemeth
>Assignee: Siddharth Ahuja
>Priority: Major
> Attachments: YARN-9996.001.patch
>
>
> Method 'isMutationAllowed' contains many uses of substring and lastIndexOf.
> These could be extracted and simplified.
> Also, some logging could be added as well.
--
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] [Comment Edited] (YARN-9996) Code cleanup in QueueAdminConfigurationMutationACLPolicy
[
https://issues.apache.org/jira/browse/YARN-9996?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17081178#comment-17081178
]
Siddharth Ahuja edited comment on YARN-9996 at 4/11/20, 6:15 AM:
-
Further details:
The overall aim of the code (as understood) is to get the last child in the
queue hierarchy that has queue information available to check for admin user
access. Until this queue information is available, keep going up in the queue
hierarchy one child at a time.
+*Before* (un-important bits extracted away):+
{code}
for each queue
{
if the queue has a child, get the last child.
Try getting the queue info for this child.
while we do not have queue info
{
get the queue minus its last child (move up in the hierarchy)
if the trimmed queue has a child, get the last child.
Try getting the queue info for this child.
}
...
}
{code}
+*After* (un-important bits extracted away):+
{code}
for each queue
{
while we do not have queue info
{
if the queue has a child, get the last child.
Try getting the queue info for this child.
if the queue has a child, trim the queue and get the queue minus its
last child (move up in the hierarchy).
}
...
}
{code}
was (Author: sahuja):
Further details:
The overall aim of the code (as understood) is to get the last child in the
queue hierarchy that has queue information available to check for admin user
access. Until this queue information is available, keep going up in the queue
hierarchy one child at a time.
+*Before* (un-important bits extracted away):+
{code}
for each queue
{
if the queue has a child, get the last child.
Try getting the queue info for this child.
while we do not have queue info
{
get the queue minus its last child (move up in the hierarchy)
if the trimmed queue has a child, get the last child.
Try getting the queue info for this child.
}
...
}
{code}
*After* (un-important bits extracted away):+
{code}
for each queue
{
while we do not have queue info
{
if the queue has a child, get the last child.
Try getting the queue info for this child.
if the queue has a child, trim the queue and get the queue minus its
last child (move up in the hierarchy).
}
...
}
{code}
> Code cleanup in QueueAdminConfigurationMutationACLPolicy
>
>
> Key: YARN-9996
> URL: https://issues.apache.org/jira/browse/YARN-9996
> Project: Hadoop YARN
> Issue Type: Improvement
>Reporter: Szilard Nemeth
>Assignee: Siddharth Ahuja
>Priority: Major
> Attachments: YARN-9996.001.patch
>
>
> Method 'isMutationAllowed' contains many uses of substring and lastIndexOf.
> These could be extracted and simplified.
> Also, some logging could be added as well.
--
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] [Comment Edited] (YARN-9996) Code cleanup in QueueAdminConfigurationMutationACLPolicy
[
https://issues.apache.org/jira/browse/YARN-9996?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17081178#comment-17081178
]
Siddharth Ahuja edited comment on YARN-9996 at 4/11/20, 6:12 AM:
-
Further details:
The overall aim of the code (as understood) is to get the last child in the
queue hierarchy that has queue information available to check for admin user
access. Until this queue information is available, keep going up in the queue
hierarchy one child at a time.
+*Before* (un-important bits extracted away):+
{code}
for each queue
{
if the queue has a child, get the last child.
Try getting the queue info for this child.
while we do not have queue info
{
get the queue minus its last child (move up in the hierarchy)
if the trimmed queue has a child, get the last child.
Try getting the queue info for this child.
}
...
}
{code}
*After* (un-important bits extracted away):+
{code}
for each queue
{
while we do not have queue info
{
if the queue has a child, get the last child.
Try getting the queue info for this child.
if the queue has a child, trim the queue and get the queue minus its
last child (move up in the hierarchy).
}
...
}
{code}
was (Author: sahuja):
Further details:
The overall aim of the code (as understood) is to get the last child in the
queue hierarchy that has queue information available to check for admin user
access. Until this queue information is available, keep going up in the queue
hierarchy one child at a time.
+*Before* (un-important bits extracted away):+
{code}
for each queue
{
if the queue has a child, get the last child.
Try getting the queue info for this child.
while we do not have queue info
{
get the queue minus its last child (move up in the hierarchy)
if the trimmed queue has a child, get the last child.
Try getting the queue info for this child.
}
...
}
{code}
*After*(un-important bits extracted away):+
{code}
for each queue
{
while we do not have queue info
{
if the queue has a child, get the last child.
Try getting the queue info for this child.
if the queue has a child, trim the queue and get the queue minus its
last child (move up in the hierarchy).
}
...
}
{code}
> Code cleanup in QueueAdminConfigurationMutationACLPolicy
>
>
> Key: YARN-9996
> URL: https://issues.apache.org/jira/browse/YARN-9996
> Project: Hadoop YARN
> Issue Type: Improvement
>Reporter: Szilard Nemeth
>Assignee: Siddharth Ahuja
>Priority: Major
> Attachments: YARN-9996.001.patch
>
>
> Method 'isMutationAllowed' contains many uses of substring and lastIndexOf.
> These could be extracted and simplified.
> Also, some logging could be added as well.
--
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] [Comment Edited] (YARN-9996) Code cleanup in QueueAdminConfigurationMutationACLPolicy
[
https://issues.apache.org/jira/browse/YARN-9996?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17081178#comment-17081178
]
Siddharth Ahuja edited comment on YARN-9996 at 4/11/20, 6:12 AM:
-
Further details:
The overall aim of the code (as understood) is to get the last child in the
queue hierarchy that has queue information available to check for admin user
access. Until this queue information is available, keep going up in the queue
hierarchy one child at a time.
+*Before* (un-important bits extracted away):+
{code}
for each queue
{
if the queue has a child, get the last child.
Try getting the queue info for this child.
while we do not have queue info
{
get the queue minus its last child (move up in the hierarchy)
if the trimmed queue has a child, get the last child.
Try getting the queue info for this child.
}
...
}
{code}
*After*(un-important bits extracted away):+
{code}
for each queue
{
while we do not have queue info
{
if the queue has a child, get the last child.
Try getting the queue info for this child.
if the queue has a child, trim the queue and get the queue minus its
last child (move up in the hierarchy).
}
...
}
{code}
was (Author: sahuja):
Further details:
The overall aim of the code (as understood) is to get the last child in the
queue hierarchy that has queue information available to check for admin user
access. Until this queue information is available, keep going up in the queue
hierarchy one child at a time.
+*Before* (un-important bits extracted away):+
{code}
for each queue
{
if the queue has a child, get the last child.
Try getting the queue info for this child.
while we do not have queue info
{
get the queue minus its last child (move up in the hierarchy)
if the trimmed queue has a child, get the last child.
Try getting the queue info for this child.
}
...
}
{code}
After*(un-important bits extracted away):+
{code}
for each queue
{
while we do not have queue info
{
if the queue has a child, get the last child.
Try getting the queue info for this child.
if the queue has a child, trim the queue and get the queue minus its
last child (move up in the hierarchy).
}
...
}
{code}
> Code cleanup in QueueAdminConfigurationMutationACLPolicy
>
>
> Key: YARN-9996
> URL: https://issues.apache.org/jira/browse/YARN-9996
> Project: Hadoop YARN
> Issue Type: Improvement
>Reporter: Szilard Nemeth
>Assignee: Siddharth Ahuja
>Priority: Major
> Attachments: YARN-9996.001.patch
>
>
> Method 'isMutationAllowed' contains many uses of substring and lastIndexOf.
> These could be extracted and simplified.
> Also, some logging could be added as well.
--
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] [Comment Edited] (YARN-9996) Code cleanup in QueueAdminConfigurationMutationACLPolicy
[
https://issues.apache.org/jira/browse/YARN-9996?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17081178#comment-17081178
]
Siddharth Ahuja edited comment on YARN-9996 at 4/11/20, 6:12 AM:
-
Further details:
The overall aim of the code (as understood) is to get the last child in the
queue hierarchy that has queue information available to check for admin user
access. Until this queue information is available, keep going up in the queue
hierarchy one child at a time.
+*Before* (un-important bits extracted away):+
{code}
for each queue
{
if the queue has a child, get the last child.
Try getting the queue info for this child.
while we do not have queue info
{
get the queue minus its last child (move up in the hierarchy)
if the trimmed queue has a child, get the last child.
Try getting the queue info for this child.
}
...
}
{code}
After*(un-important bits extracted away):+
{code}
for each queue
{
while we do not have queue info
{
if the queue has a child, get the last child.
Try getting the queue info for this child.
if the queue has a child, trim the queue and get the queue minus its
last child (move up in the hierarchy).
}
...
}
{code}
was (Author: sahuja):
Further details:
The overall aim of the code (as understood) is to get the last child in the
queue hierarchy that has queue information available to check for admin user
access. Until this queue information is available, keep going up in the queue
hierarchy one child at a time.
+*Before* (un-important bits extracted away):+
{code}
for each queue
{
if the queue has a child, get the last child.
Try getting the queue info for this child.
while we do not have queue info
{
get the queue minus its last child (move up in the hierarchy)
if the trimmed queue has a child, get the last child.
Try getting the queue info for this child.
}
...
}
After*(un-important bits extracted away):+
{code}
for each queue
{
while we do not have queue info
{
if the queue has a child, get the last child.
Try getting the queue info for this child.
if the queue has a child, trim the queue and get the queue minus its
last child (move up in the hierarchy).
}
...
}
{code}
> Code cleanup in QueueAdminConfigurationMutationACLPolicy
>
>
> Key: YARN-9996
> URL: https://issues.apache.org/jira/browse/YARN-9996
> Project: Hadoop YARN
> Issue Type: Improvement
>Reporter: Szilard Nemeth
>Assignee: Siddharth Ahuja
>Priority: Major
> Attachments: YARN-9996.001.patch
>
>
> Method 'isMutationAllowed' contains many uses of substring and lastIndexOf.
> These could be extracted and simplified.
> Also, some logging could be added as well.
--
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] [Comment Edited] (YARN-9996) Code cleanup in QueueAdminConfigurationMutationACLPolicy
[
https://issues.apache.org/jira/browse/YARN-9996?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17081178#comment-17081178
]
Siddharth Ahuja edited comment on YARN-9996 at 4/11/20, 6:11 AM:
-
Further details:
The overall aim of the code (as understood) is to get the last child in the
queue hierarchy that has queue information available to check for admin user
access. Until this queue information is available, keep going up in the queue
hierarchy one child at a time.
+*Before* (un-important bits extracted away):+
{code}
for each queue
{
if the queue has a child, get the last child.
Try getting the queue info for this child.
while we do not have queue info
{
get the queue minus its last child (move up in the hierarchy)
if the trimmed queue has a child, get the last child.
Try getting the queue info for this child.
}
...
}
After*(un-important bits extracted away):+
{code}
for each queue
{
while we do not have queue info
{
if the queue has a child, get the last child.
Try getting the queue info for this child.
if the queue has a child, trim the queue and get the queue minus its
last child (move up in the hierarchy).
}
...
}
{code}
was (Author: sahuja):
Further details:
The overall aim of the code (as understood) is to get the last child in the
queue hierarchy that has queue information available to check for admin user
access. Until this queue information is available, keep going up in the queue
hierarchy one child at a time.
+*Before *(un-important bits extracted away):+
{code}
for each queue
{
if the queue has a child, get the last child.
Try getting the queue info for this child.
while we do not have queue info
{
get the queue minus its last child (move up in the hierarchy)
if the trimmed queue has a child, get the last child.
Try getting the queue info for this child.
}
...
}
+*After*(un-important bits extracted away):+
{code}
for each queue
{
while we do not have queue info
{
if the queue has a child, get the last child.
Try getting the queue info for this child.
if the queue has a child, trim the queue and get the queue minus its
last child (move up in the hierarchy).
}
...
}
{code}
> Code cleanup in QueueAdminConfigurationMutationACLPolicy
>
>
> Key: YARN-9996
> URL: https://issues.apache.org/jira/browse/YARN-9996
> Project: Hadoop YARN
> Issue Type: Improvement
>Reporter: Szilard Nemeth
>Assignee: Siddharth Ahuja
>Priority: Major
> Attachments: YARN-9996.001.patch
>
>
> Method 'isMutationAllowed' contains many uses of substring and lastIndexOf.
> These could be extracted and simplified.
> Also, some logging could be added as well.
--
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
