[
https://issues.apache.org/jira/browse/YARN-10425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17270425#comment-17270425
]
Ahmed Hussein edited comment on YARN-10425 at 1/22/21, 8:57 PM:
----------------------------------------------------------------
[~shuzirra], [~pbacsko], [~snemeth], [~BilwaST] thanks for the contribution.
I have a question about the changes introduced by this Ticket. The following
code block
is from
[CSMappingPlacementRule#L128|https://github.com/apache/hadoop/commit/567600fd80896c1c9b0db1f228368d4eb2a694a2#diff-92b5797cf7739d330364d967172e65e61a859c776d9ebe526aba03ea33039033R127]
{code:java}
if (groups == null) {
//We cannot use Groups#getUserToGroupsMappingService here, because when
//tests change the HADOOP_SECURITY_GROUP_MAPPING, Groups won't refresh its
//cached instance of groups, so we might get a Group instance which
//ignores the HADOOP_SECURITY_GROUP_MAPPING settings.
groups = new Groups(conf);
}
{code}
IIUC, the design of groups caching "{{Groups.cache}}" relies on the fact that
the Groups being a singleton. Otherwise, there will be inconsistent behavior
especially in classes like {{JniBasedUnixGroupsNetgroupMapping}} and
{{ShellBasedUnixGroupsNetgroupMapping}}. Both mapping implementations have a
second caching layer for the netgroups "{{NetgroupCache}}".
I have the following two concerns regarding an independent Groups instance in
{{CSMappingPlacementRule.java}}
* It breaks the design leading to inconsistent behaviors that do not match the
expected. As I mentioned, {{NetgroupCache}} contents won't be defined.
* Performance considerations. Allocating "N" instances of {{Groups}} means
fetching the user's groups "N" times. Therefore, Guava cacheLoader's refresh
will be done "N" times, and so on.
Why did you decide to make that change instead of fixing the design of the unit
tests?
IIUC, there is a need to fix that bug in a follow up Jira.
was (Author: ahussein):
[~shuzirra], [~pbacsko] thanks for the contribution.
I have a question about the changes introduced by this Ticket. The following
code block
is from
[CSMappingPlacementRule#L128|https://github.com/apache/hadoop/commit/567600fd80896c1c9b0db1f228368d4eb2a694a2#diff-92b5797cf7739d330364d967172e65e61a859c776d9ebe526aba03ea33039033R127]
{code:java}
if (groups == null) {
//We cannot use Groups#getUserToGroupsMappingService here, because when
//tests change the HADOOP_SECURITY_GROUP_MAPPING, Groups won't refresh its
//cached instance of groups, so we might get a Group instance which
//ignores the HADOOP_SECURITY_GROUP_MAPPING settings.
groups = new Groups(conf);
}
{code}
IIUC, the design of groups caching "{{Groups.cache}}" relies on the fact that
the Groups being a singleton. Otherwise, there will be inconsistent behavior
especially in classes like {{JniBasedUnixGroupsNetgroupMapping}} and
{{ShellBasedUnixGroupsNetgroupMapping}}. Both mapping implementations have a
second caching layer for the netgroups "{{NetgroupCache}}".
I have the following two concerns regarding an independent Groups instance in
{{CSMappingPlacementRule.java}}
* It breaks the design leading to inconsistent behaviors that do not match the
expected. As I mentioned, {{NetgroupCache}} contents won't be defined.
* Performance considerations. Allocating "N" instances of {{Groups}} means
fetching the user's groups "N" times. Therefore, Guava cacheLoader's refresh
will be done "N" times, and so on.
Why did you decide to make that change instead of fixing the design of the unit
tests?
IIUC, there is a need to fix that bug in a follow up Jira.
> Replace the legacy placement engine in CS with the new one
> ----------------------------------------------------------
>
> Key: YARN-10425
> URL: https://issues.apache.org/jira/browse/YARN-10425
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Gergely Pollak
> Assignee: Gergely Pollak
> Priority: Major
> Fix For: 3.4.0
>
> Attachments: YARN-10425.001.patch, YARN-10425.002.patch,
> YARN-10425.003.patch, YARN-10425.004.patch, YARN-10425.005.patch,
> YARN-10425.006.patch, YARN-10425.007.patch
>
>
> Remove the UserGroupMapping and ApplicationName mapping classes, and use the
> new CSMappingPlacementRule instead. Also cleanup the orphan classes which are
> used by these classes only.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
