Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not log config when the config is not changed #694

Merged
merged 1 commit into from
Sep 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/config/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package config

import (
"bytes"
"maps"
"net"
"os"
"path/filepath"
Expand Down Expand Up @@ -168,6 +169,7 @@ func NewConfig() *Config {

func (cfg *Config) Clone() *Config {
newCfg := *cfg
newCfg.Labels = maps.Clone(cfg.Labels)
return &newCfg
}

Expand Down
9 changes: 9 additions & 0 deletions lib/config/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,3 +170,12 @@ func TestGetIPPort(t *testing.T) {
require.Equal(t, cas.port, statusPort)
}
}

func TestCloneConfig(t *testing.T) {
cfg := testProxyConfig
cfg.Labels = map[string]string{"a": "b"}
clone := cfg.Clone()
require.Equal(t, cfg, *clone)
cfg.Labels["c"] = "d"
require.NotContains(t, clone.Labels, "c")
}
23 changes: 12 additions & 11 deletions pkg/manager/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,7 @@ func (e *ConfigManager) reloadConfigFile(file string) error {
// that are specified by users.
func (e *ConfigManager) SetTOMLConfig(data []byte) (err error) {
e.sts.Lock()
defer func() {
if err == nil {
e.logger.Info("current config", zap.Any("cfg", e.sts.current))
}
e.sts.Unlock()
}()
defer e.sts.Unlock()

base := e.sts.current
if base == nil {
Expand All @@ -62,16 +57,22 @@ func (e *ConfigManager) SetTOMLConfig(data []byte) (err error) {
}

e.sts.current = base
originalData := e.sts.data
var buf bytes.Buffer
if err = toml.NewEncoder(&buf).Encode(base); err != nil {
return errors.WithStack(err)
}
e.sts.checksum = crc32.ChecksumIEEE(buf.Bytes())

for _, list := range e.sts.listeners {
list <- base.Clone()
newData := buf.Bytes()

// TiDB-Operator may set the same labels by the HTTP API periodically, don't notify the listeners every time.
if originalData == nil || !bytes.Equal(originalData, newData) {
e.sts.checksum = crc32.ChecksumIEEE(newData)
e.sts.data = newData
e.logger.Info("current config", zap.Any("cfg", e.sts.current))
for _, list := range e.sts.listeners {
list <- base.Clone()
}
}

return
}

Expand Down
11 changes: 9 additions & 2 deletions pkg/manager/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"os"
"path/filepath"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -142,7 +143,7 @@ func TestConfigRemove(t *testing.T) {
require.NoError(t, os.WriteFile(tmpcfg, []byte(`proxy.addr = "gg"`), 0644))

// check that re-watch still works
require.Eventually(t, func() bool { return cfgmgr.GetConfig().Proxy.Addr == "gg" }, 3*time.Second, 100*time.Millisecond)
require.Eventually(t, func() bool { return cfgmgr.GetConfig() != nil && cfgmgr.GetConfig().Proxy.Addr == "gg" }, 3*time.Second, 100*time.Millisecond)

// remove again but with a long sleep
require.NoError(t, os.Remove(tmpcfg))
Expand Down Expand Up @@ -298,13 +299,19 @@ func TestFilePath(t *testing.T) {
}

func TestChecksum(t *testing.T) {
cfgmgr, _, _ := testConfigManager(t, "", "")
cfgmgr, text, _ := testConfigManager(t, "", "")
require.Equal(t, 1, strings.Count(text.String(), "current config"))
c1 := cfgmgr.GetConfigChecksum()
require.NoError(t, cfgmgr.SetTOMLConfig([]byte(`proxy.addr = "gg"`)))
require.Equal(t, 2, strings.Count(text.String(), "current config"))
// same config, shouldn't log it again
require.NoError(t, cfgmgr.SetTOMLConfig([]byte(`proxy.addr = "gg"`)))
require.Equal(t, 2, strings.Count(text.String(), "current config"))
c2 := cfgmgr.GetConfigChecksum()
require.NoError(t, cfgmgr.SetTOMLConfig([]byte(`proxy.addr = "vv"`)))
c3 := cfgmgr.GetConfigChecksum()
require.NoError(t, cfgmgr.SetTOMLConfig([]byte(`proxy.addr="gg"`)))
require.Equal(t, 4, strings.Count(text.String(), "current config"))
c4 := cfgmgr.GetConfigChecksum()
require.Equal(t, c2, c4)
require.NotEqual(t, c1, c2)
Expand Down
3 changes: 2 additions & 1 deletion pkg/manager/config/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,12 @@ type ConfigManager struct {
kv *btree.BTreeG[KVValue]

checkFileInterval time.Duration
fileContent []byte
fileContent []byte // used to compare whether the config file has changed
sts struct {
sync.Mutex
listeners []chan<- *config.Config
current *config.Config
data []byte // used to strictly compare whether the config has changed
checksum uint32 // checksum of the unmarshalled toml
}
}
Expand Down
4 changes: 1 addition & 3 deletions pkg/server/api/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,8 @@ func (h *Server) attachLogger(c *gin.Context) {
switch {
case len(c.Errors) > 0:
h.lg.Warn(path, fields...)
case strings.Contains(path, "/debug"), strings.Contains(path, "/metrics"):
h.lg.Debug(path, fields...)
default:
h.lg.Info(path, fields...)
h.lg.Debug(path, fields...)
}
}

Expand Down