From 8b155e34c722d8952adf84c5ba0fc994443992cf Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Fri, 17 Dec 2021 20:15:17 +0530 Subject: [PATCH] Re-initialize plugins only when needed (#19158) Previously, for every config change, we would re-initialize plugins irrespective of whether there would be any actual plugin related changed or not. Now, we compute the difference between previous and new configs and check whether there has been any changes to plugin related configuration, and re-init plugins only if that's true. This improves performance as plugin re-initialization is quite costly. https://community-daily.mattermost.com/boards/workspace/zyoahc9uapdn3xdptac6jb69ic/285b80a3-257d-41f6-8cf4-ed80ca9d92e5/495cdb4d-c13a-4992-8eb9-80cfee2819a4/bed59298-27da-4b74-8be5-cdeb4c50c3e2 ```release-note NONE ``` --- app/channels.go | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/app/channels.go b/app/channels.go index 03b36fceca..672223920c 100644 --- a/app/channels.go +++ b/app/channels.go @@ -5,15 +5,18 @@ package app import ( "runtime" + "strings" "sync" "sync/atomic" "github.com/mattermost/mattermost-server/v6/app/imaging" "github.com/mattermost/mattermost-server/v6/app/request" + "github.com/mattermost/mattermost-server/v6/config" "github.com/mattermost/mattermost-server/v6/einterfaces" "github.com/mattermost/mattermost-server/v6/model" "github.com/mattermost/mattermost-server/v6/plugin" "github.com/mattermost/mattermost-server/v6/services/imageproxy" + "github.com/mattermost/mattermost-server/v6/shared/mlog" "github.com/pkg/errors" ) @@ -119,11 +122,32 @@ func (ch *Channels) Start() error { ch.initPlugins(ctx, *ch.srv.Config().PluginSettings.Directory, *ch.srv.Config().PluginSettings.ClientDirectory) ch.AddConfigListener(func(prevCfg, cfg *model.Config) { - if *cfg.PluginSettings.Enable { - ch.initPlugins(ctx, *cfg.PluginSettings.Directory, *ch.srv.Config().PluginSettings.ClientDirectory) - } else { - ch.ShutDownPlugins() + // We compute the difference between configs + // to ensure we don't re-init plugins unnecessarily. + diffs, err := config.Diff(prevCfg, cfg) + if err != nil { + mlog.Warn("Error in comparing configs", mlog.Err(err)) + return } + + hasDiff := false + // TODO: This could be a method on ConfigDiffs itself + for _, diff := range diffs { + if strings.HasPrefix(diff.Path, "PluginSettings.") { + hasDiff = true + break + } + } + + // Do only if some plugin related settings has changed. + if hasDiff { + if *cfg.PluginSettings.Enable { + ch.initPlugins(ctx, *cfg.PluginSettings.Directory, *ch.srv.Config().PluginSettings.ClientDirectory) + } else { + ch.ShutDownPlugins() + } + } + }) if err := ch.ensureAsymmetricSigningKey(); err != nil {