Wangda Tan, thank you very much for your in-depth review and comments.
1) When there're several active users with [combined sum of] weights < 1. ... However in this implementation ... a1 can get all queue's resource (because #active-user-applied-weights = 1/0.3) while a2 got starved.
No, that's not how it will work with this implementation.
Sunil G had a similar question above. Having a combined sum of weights < 1 works because userLimitResource (the return value of computeUserLimit) is only ever used by getComputedResourceLimitFor[Active|All]Users, which multiplies the value of userLimitResource by the appropriate user weight(s). This will result in the correct value of userLimit for each specific user. If the sum of active user(s)'s weight(s) is < 1, then it is true that userLimitResource is larger than the actual user limit, and sometimes even larger than the actual number of resources used. However, this algorithm calculates userLimit correctly and consistently when getComputedResourceLimitFor[Active|All]Users multiplies it by each user's weight.
2) I would like to prevent setting user's weight to <= 0.
Instead of a warning, I will cause the parse of the CS config to fail if weight is < 0. I would like Jason Lowe's and Nathan Roberts's feedback on whether or not weight == 0 is reasonable and consistent.
Generally speaking, set user weight < 1 is a reasonable requirement however I don't think we're ready for that. It looks there're bunch of things we need to do to make #2 and related preemption logic works properly.
I am afraid that I disagree for reasons stated above. #2 can be addressed with a simple check that treats failure the same as other parsing issues. The one concern that remains in my mind is to ensure that this algorithm calculates allUserLimit correctly for preemption. I have not yet combined and tested this patch with the one for
YARN-2113. I will do so and post my findings.
Beyond that, I suggest to make #active-users-times-weight can updated in O(1) for every changes to active users set or any active user's weight get updated.
Yes, good point. Although #active-users-times-weight and #users-times-weight are only calculated in computeUserLimit, and computeUserLimit is only called when a significant event happens, we could eliminate the need to calculate this for things like container allocate and container free events. I will modify the patch to do this.
Also, weight of users applies to hard limit of user (user limit factor) as well. This is a gray area to me, since it may cause some issue of resource planning (one more factor apply to maximum resource of user). Would like to hear thoughts from Jason Lowe/Sunil G as well.
I look forward to Jason Lowe's and Sunil G's comments