pbacsko commented on code in PR #758:
URL: https://github.com/apache/yunikorn-core/pull/758#discussion_r1442930284
##########
pkg/scheduler/ugm/manager.go:
##########
@@ -401,6 +393,62 @@ func (m *Manager) clearEarlierSetLimits(newUserLimits
map[string]map[string]*Lim
m.clearEarlierSetUserLimits(newUserLimits)
}
+// clearEarlierSetUserWildCardLimits Traverse new wild card user config and
decide whether earlier usage needs to be cleared/updated or not
+// by comparing with the existing config. If config set earlier but not now,
then traverse all users, check whether wild card limit has been applied/used or
not.
+// Reset earlier settings for the users only when wild card user limit has
been applied.
+// If config set earlier and now as well, then traverse all users and apply
the current wild card user limit configs
+// only when wild card user limit has been applied earlier.
+func (m *Manager) clearEarlierSetUserWildCardLimits(newUserWildCardLimits
map[string]*LimitConfig, newUserLimits map[string]map[string]*LimitConfig) {
+ m.RLock()
+ defer m.RUnlock()
+ for queuePath, currentLimitConfig := range m.userWildCardLimitsConfig {
+ hierarchy := strings.Split(queuePath, configs.DOT)
+ _, currentQPExists := m.userLimits[queuePath]
+ _, newQPExists := newUserLimits[queuePath]
+
+ // Is queue path exists? In case wild card user limit not
exists, reset limit settings, useWildCard flag etc for all those users
Review Comment:
Nit: "Does queue path exist?", "In case wild limit does not exist"
##########
pkg/scheduler/ugm/manager.go:
##########
@@ -401,6 +393,62 @@ func (m *Manager) clearEarlierSetLimits(newUserLimits
map[string]map[string]*Lim
m.clearEarlierSetUserLimits(newUserLimits)
}
+// clearEarlierSetUserWildCardLimits Traverse new wild card user config and
decide whether earlier usage needs to be cleared/updated or not
+// by comparing with the existing config. If config set earlier but not now,
then traverse all users, check whether wild card limit has been applied/used or
not.
+// Reset earlier settings for the users only when wild card user limit has
been applied.
+// If config set earlier and now as well, then traverse all users and apply
the current wild card user limit configs
+// only when wild card user limit has been applied earlier.
+func (m *Manager) clearEarlierSetUserWildCardLimits(newUserWildCardLimits
map[string]*LimitConfig, newUserLimits map[string]map[string]*LimitConfig) {
+ m.RLock()
+ defer m.RUnlock()
+ for queuePath, currentLimitConfig := range m.userWildCardLimitsConfig {
+ hierarchy := strings.Split(queuePath, configs.DOT)
+ _, currentQPExists := m.userLimits[queuePath]
+ _, newQPExists := newUserLimits[queuePath]
+
+ // Is queue path exists? In case wild card user limit not
exists, reset limit settings, useWildCard flag etc for all those users
+ if newLimitConfig, ok := newUserWildCardLimits[queuePath]; !ok
&& (!currentQPExists || !newQPExists) {
+ for _, ut := range m.userTrackers {
+ _, exists :=
m.userLimits[queuePath][ut.userName]
+ if _, ok =
newUserLimits[queuePath][ut.userName]; !ok || !exists {
+ log.Log(log.SchedUGM).Debug("Need to
clear earlier set configs for user because wild card limit has been applied
earlier",
+ zap.String("user", ut.userName),
+ zap.String("queue path",
queuePath))
+ ut.setLimits(hierarchy, nil, 0, false,
true)
+ }
+ }
+ } else if !currentQPExists || !newQPExists {
+ // In case wild card user limit exists, compare the old
wild card limits with new limits for existing users already using wild card
limits.
+ // In case of difference, set new limits for all those
users.
+ if currentLimitConfig.maxApplications !=
newLimitConfig.maxApplications ||
+
!resources.Equals(currentLimitConfig.maxResources, newLimitConfig.maxResources)
{
+ for _, ut := range m.userTrackers {
+ log.Log(log.SchedUGM).Debug("Need to
update earlier set configs for user because wild card limit applied earlier has
been updated",
+ zap.String("user", ut.userName),
+ zap.String("queue path",
queuePath))
+ _, exists :=
m.userLimits[queuePath][ut.userName]
+ if _, ok =
newUserLimits[queuePath][ut.userName]; !ok || !exists {
+ ut.setLimits(hierarchy,
newLimitConfig.maxResources, newLimitConfig.maxApplications, true, true)
+ }
Review Comment:
L421-432 codecov warning - please check this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
