[ 
https://issues.apache.org/jira/browse/YARN-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15850294#comment-15850294
 ] 

Wangda Tan commented on YARN-5889:
----------------------------------

Hi Sunil,

bq. I am slightly confused here. I think we might need null check. I ll help to 
share detailed view for that. ...
I think the latest patch do the correct thing, except one thing:

Instead of doing:

{code}
    Map<String, Map<SchedulingMode, Resource>> userMap = (isActive)
        ? preComputedActiveUserLimit
        : preComputedAllUserLimit;

    return !userMap.containsKey(nodePartition)
        || (getLocalVersionOfUsersState(nodePartition, schedulingMode,
            isActive) != latestVersionOfUserCount);
{code}

I think it should be enough to do:

{code}
return getLocalVersionOfUsersState(nodePartition, schedulingMode,
            isActive) != latestVersionOfUserCount;
{code}

The reason is, getLocalVersionOfUsersState will always return -1 when userMap 
doesn't contains nodePartition.
And {{reComputeUserLimits}} will insert the map and return.

Is my understand correct?

bq. userLimitNeedsRecompute or getLatestVersionOfUsersState are not writeLock 
protected ...

{{userLimitNeedsRecompute}} is now writeLock-protected (I think you don't need 
atomicLong any more but it's fine to keep it as-is). And usage of 
latestVersionOfUsersState is always under read/writeLock. So 
latestVersionOfUsersState.get() is always up-to-date value and will not be 
changed during calculation (it is also fine if it is changed during usage, our 
goal is to make local cache invalidated).

And one additional minor comment:
1) computeUserLimit:
- Remove {{User user}} from parameter list, and instead of calling 
lQueue.getUser, it's better to call this.getUser()

+1 to the latest patch beyond above comments, I think the patch is ready-to-go 
once Jenkins give a +1. 

> Improve user-limit calculation in capacity scheduler
> ----------------------------------------------------
>
>                 Key: YARN-5889
>                 URL: https://issues.apache.org/jira/browse/YARN-5889
>             Project: Hadoop YARN
>          Issue Type: Bug
>          Components: capacity scheduler
>            Reporter: Sunil G
>            Assignee: Sunil G
>         Attachments: YARN-5889.0001.patch, 
> YARN-5889.0001.suggested.patchnotes, YARN-5889.0002.patch, 
> YARN-5889.0003.patch, YARN-5889.0004.patch, YARN-5889.0005.patch, 
> YARN-5889.0006.patch, YARN-5889.0007.patch, YARN-5889.0008.patch, 
> YARN-5889.0009.patch, YARN-5889.v0.patch, YARN-5889.v1.patch, 
> YARN-5889.v2.patch
>
>
> Currently user-limit is computed during every heartbeat allocation cycle with 
> a write lock. To improve performance, this tickets is focussing on moving 
> user-limit calculation out of heartbeat allocation flow.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to