This is an automated email from the ASF dual-hosted git repository.

chia7712 pushed a commit to branch branch-1.5
in repository https://gitbox.apache.org/repos/asf/yunikorn-k8shim.git


The following commit(s) were added to refs/heads/branch-1.5 by this push:
     new fb4e3f11 [YUNIKORN-2630] Release context lock early for config changes 
(#842)
fb4e3f11 is described below

commit fb4e3f11345e6a9866dfaea97770c94b9421807b
Author: Wilfred Spiegelenburg <wilfr...@apache.org>
AuthorDate: Tue May 21 22:35:18 2024 +0800

    [YUNIKORN-2630] Release context lock early for config changes (#842)
    
    Release the lock of the context in the shim when processing is done.
    When the config changes are sent to the core the k8shim should not be
    locked. The context changes have been finalised at that point.
    
    The core handles its own locking and serialises config changes that come
    in from the k8shim.
    review: remove call through api to get config.
    
    Closes: #842
    
    Signed-off-by: Chia-Ping Tsai <chia7...@gmail.com>
---
 pkg/cache/context.go      | 39 +++++++++++++++++++++++----------------
 pkg/conf/schedulerconf.go |  6 ++++++
 2 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/pkg/cache/context.go b/pkg/cache/context.go
index bea59346..199ab128 100644
--- a/pkg/cache/context.go
+++ b/pkg/cache/context.go
@@ -605,40 +605,47 @@ func (ctx *Context) deletePriorityClass(obj interface{}) {
 }
 
 func (ctx *Context) triggerReloadConfig(index int, configMap *v1.ConfigMap) {
-       ctx.lock.Lock()
-       defer ctx.lock.Unlock()
-
-       conf := ctx.apiProvider.GetAPIs().GetConf()
-       if !conf.EnableConfigHotRefresh {
+       // hot reload is turned off do nothing
+       // hot reload can be turned off by an update: safety first access under 
lock to prevent data race
+       if !schedulerconf.GetSchedulerConf().IsConfigReloadable() {
                log.Log(log.ShimContext).Info("hot-refresh disabled, skipping 
scheduler configuration update")
                return
        }
-
-       ctx.configMaps[index] = configMap
-       err := schedulerconf.UpdateConfigMaps(ctx.configMaps, false)
-       if err != nil {
-               log.Log(log.ShimContext).Error("Unable to update configmap, 
ignoring changes", zap.Error(err))
+       // update the maps in the context: return on failure, logged in the 
called method
+       confMap := ctx.setConfigMap(index, configMap)
+       if confMap == nil {
                return
        }
-
-       confMap := schedulerconf.FlattenConfigMaps(ctx.configMaps)
-
-       conf = ctx.apiProvider.GetAPIs().GetConf()
        log.Log(log.ShimContext).Info("reloading scheduler configuration")
        config := utils.GetCoreSchedulerConfigFromConfigMap(confMap)
        extraConfig := utils.GetExtraConfigFromConfigMap(confMap)
 
        request := &si.UpdateConfigurationRequest{
-               RmID:        conf.ClusterID,
-               PolicyGroup: conf.PolicyGroup,
+               RmID:        schedulerconf.GetSchedulerConf().ClusterID,
+               PolicyGroup: schedulerconf.GetSchedulerConf().PolicyGroup,
                Config:      config,
                ExtraConfig: extraConfig,
        }
+       // tell the core to update: sync call that is serialised on the core 
side
        if err := 
ctx.apiProvider.GetAPIs().SchedulerAPI.UpdateConfiguration(request); err != nil 
{
                log.Log(log.ShimContext).Error("reload configuration failed", 
zap.Error(err))
        }
 }
 
+// setConfigMap sets the new config map object in the list of maps maintained 
in the context and returns a flat map
+// of the settings from both maps
+func (ctx *Context) setConfigMap(index int, configMap *v1.ConfigMap) 
map[string]string {
+       ctx.lock.Lock()
+       defer ctx.lock.Unlock()
+       ctx.configMaps[index] = configMap
+       err := schedulerconf.UpdateConfigMaps(ctx.configMaps, false)
+       if err != nil {
+               log.Log(log.ShimContext).Error("Unable to update configmap, 
ignoring changes", zap.Error(err))
+               return nil
+       }
+       return schedulerconf.FlattenConfigMaps(ctx.configMaps)
+}
+
 // EventsToRegister returns the Kubernetes events that should be watched for 
updates which may effect predicate processing
 func (ctx *Context) EventsToRegister(queueingHintFn framework.QueueingHintFn) 
[]framework.ClusterEventWithHint {
        return ctx.predManager.EventsToRegister(queueingHintFn)
diff --git a/pkg/conf/schedulerconf.go b/pkg/conf/schedulerconf.go
index da8336a0..2e941213 100644
--- a/pkg/conf/schedulerconf.go
+++ b/pkg/conf/schedulerconf.go
@@ -269,6 +269,12 @@ func (conf *SchedulerConf) IsTestMode() bool {
        return conf.TestMode
 }
 
+func (conf *SchedulerConf) IsConfigReloadable() bool {
+       conf.RLock()
+       defer conf.RUnlock()
+       return conf.EnableConfigHotRefresh
+}
+
 func (conf *SchedulerConf) GetSchedulingInterval() time.Duration {
        conf.RLock()
        defer conf.RUnlock()


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@yunikorn.apache.org
For additional commands, e-mail: issues-h...@yunikorn.apache.org

Reply via email to