From a72acd21b0ef17d6cd9064dd105042b6edb3b8dc Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Thu, 3 Mar 2022 21:41:51 -0700 Subject: core: Retry dynamic config load if config unchanged (see discussion in #4603) --- admin.go | 2 +- caddy.go | 45 ++++++++++++++++++++++++++++++++------------- 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/admin.go b/admin.go index 8558cee..d31308a 100644 --- a/admin.go +++ b/admin.go @@ -938,7 +938,7 @@ func handleConfig(w http.ResponseWriter, r *http.Request) error { forceReload := r.Header.Get("Cache-Control") == "must-revalidate" err := changeConfig(r.Method, r.URL.Path, body, forceReload) - if err != nil { + if err != nil && !errors.Is(err, errSameConfig) { return err } diff --git a/caddy.go b/caddy.go index 661d0b0..0217894 100644 --- a/caddy.go +++ b/caddy.go @@ -18,6 +18,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "log" @@ -110,14 +111,19 @@ func Load(cfgJSON []byte, forceReload bool) error { } }() - return changeConfig(http.MethodPost, "/"+rawConfigKey, cfgJSON, forceReload) + err := changeConfig(http.MethodPost, "/"+rawConfigKey, cfgJSON, forceReload) + if errors.Is(err, errSameConfig) { + err = nil // not really an error + } + return err } // changeConfig changes the current config (rawCfg) according to the // method, traversed via the given path, and uses the given input as // the new value (if applicable; i.e. "DELETE" doesn't have an input). // If the resulting config is the same as the previous, no reload will -// occur unless forceReload is true. This function is safe for +// occur unless forceReload is true. If the config is unchanged and not +// forcefully reloaded, then errConfigUnchanged This function is safe for // concurrent use. func changeConfig(method, path string, input []byte, forceReload bool) error { switch method { @@ -149,7 +155,7 @@ func changeConfig(method, path string, input []byte, forceReload bool) error { // if nothing changed, no need to do a whole reload unless the client forces it if !forceReload && bytes.Equal(rawCfgJSON, newCfg) { Log().Info("config is unchanged") - return nil + return errSameConfig } // find any IDs in this config and index them @@ -492,39 +498,47 @@ func finishSettingUp(ctx Context, cfg *Config) error { zap.String("module", val.(Module).CaddyModule().ID.Name()), zap.Int("load_delay", int(cfg.Admin.Config.LoadDelay))) - runLoadedConfig := func(config []byte) { + runLoadedConfig := func(config []byte) error { logger.Info("applying dynamically-loaded config") err := changeConfig(http.MethodPost, "/"+rawConfigKey, config, false) - if err == nil { - logger.Info("successfully applied dynamically-loaded config") - } else { + if errors.Is(err, errSameConfig) { + return err + } + if err != nil { logger.Error("failed to run dynamically-loaded config", zap.Error(err)) + return err } + logger.Info("successfully applied dynamically-loaded config") + return nil } if cfg.Admin.Config.LoadDelay > 0 { go func() { - // the loop is here only to iterate if there is an error or a no-op config load, - // in which case we simply wait the delay and try again + // the loop is here to iterate ONLY if there is an error, a no-op config load, + // or an unchanged config; in which case we simply wait the delay and try again for { timer := time.NewTimer(time.Duration(cfg.Admin.Config.LoadDelay)) select { case <-timer.C: loadedConfig, err := val.(ConfigLoader).LoadConfig(ctx) if err != nil { - Log().Error("failed loading dynamic config; will retry", zap.Error(err)) + logger.Error("failed loading dynamic config; will retry", zap.Error(err)) continue } if loadedConfig == nil { - Log().Info("dynamically-loaded config was nil; will retry") + logger.Info("dynamically-loaded config was nil; will retry") + continue + } + err = runLoadedConfig(loadedConfig) + if errors.Is(err, errSameConfig) { + logger.Info("dynamically-loaded config was unchanged; will retry") continue } - runLoadedConfig(loadedConfig) case <-ctx.Done(): if !timer.Stop() { <-timer.C } - Log().Info("stopping dynamic config loading") + logger.Info("stopping dynamic config loading") } break } @@ -798,5 +812,10 @@ var ( rawCfgIndex map[string]string ) +// errSameConfig is returned if the new config is the same +// as the old one. This isn't usually an actual, actionable +// error; it's mostly a sentinel value. +var errSameConfig = errors.New("config is unchanged") + // ImportPath is the package import path for Caddy core. const ImportPath = "github.com/caddyserver/caddy/v2" -- cgit v1.2.3