From 501a763e12a9e4c695936618a0038b06fa4817d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20R=C3=BCger?= Date: Wed, 2 Sep 2026 14:44:07 +0200 Subject: [PATCH] web: reuse the parsed web configuration while the file is unchanged MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The configuration file is consulted on every http request so that changes are picked up without a restart. That read was an os.ReadFile plus a strict YAML unmarshal plus header validation, every time, and it happens on the scrape path as well as on every TLS handshake through GetConfigForClient. It was by far the most expensive part of serving a request: measured against the handler it wraps, the toolkit's handler cost 15.9us per request where the wrapped handler cost 0.87us. Keep the parsed configuration and reuse it while the file's modification time and size are both unchanged, replacing a read and a parse with a stat in the common case. BenchmarkGetConfig 14743 ns/op 7515 B/op 64 allocs/op # before BenchmarkGetConfig 1160 ns/op 515 B/op 3 allocs/op # after An edit is still picked up on the next request, which the existing TestConfigReloading continues to cover. The limitation worth being explicit about is that a write leaving both the modification time and the size identical is not noticed. On a filesystem whose timestamps have sub-second resolution that needs a rewrite within the same nanosecond at exactly the same length; where timestamps have one second resolution the window is a second. There is a test that pins this behaviour so it is not discovered by accident later. Failures are not cached, so a file that does not parse is reported on every request until it is fixed rather than once. Callers get a copy of the configuration struct, so one caller cannot change what the next one sees; the maps inside are shared and are only ever read. Certificates are still loaded from disk on every handshake. Caching those is a separate change and is deliberately not part of this one. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Manuel Rüger --- web/config_cache_test.go | 137 +++++++++++++++++++++++++++++++++++++++ web/tls_config.go | 67 ++++++++++++++++++- 2 files changed, 203 insertions(+), 1 deletion(-) create mode 100644 web/config_cache_test.go diff --git a/web/config_cache_test.go b/web/config_cache_test.go new file mode 100644 index 00000000..4bbe50f7 --- /dev/null +++ b/web/config_cache_test.go @@ -0,0 +1,137 @@ +// Copyright The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package web + +import ( + "os" + "path/filepath" + "testing" +) + +func writeConfigFile(t *testing.T, path, content string) { + t.Helper() + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatalf("Unable to write config: %v", err) + } +} + +func getConfigOrFail(t *testing.T, path string) *Config { + t.Helper() + c, err := getConfig(path) + if err != nil { + t.Fatalf("getConfig: %v", err) + } + return c +} + +// TestGetConfigReloadsOnChange checks that an edited configuration file is +// picked up, which is what the per-request read exists for. +func TestGetConfigReloadsOnChange(t *testing.T) { + path := filepath.Join(t.TempDir(), "web-config.yml") + + writeConfigFile(t, path, "http_server_config:\n http2: true\n") + if c := getConfigOrFail(t, path); !c.HTTPConfig.HTTP2 { + t.Error("http2 = false, expected true") + } + + writeConfigFile(t, path, "http_server_config:\n http2: false\n headers:\n X-Frame-Options: deny\n") + c := getConfigOrFail(t, path) + if c.HTTPConfig.HTTP2 { + t.Error("http2 = true after the file changed, expected false") + } + if got := c.HTTPConfig.Header["X-Frame-Options"]; got != "deny" { + t.Errorf("X-Frame-Options = %q after the file changed, expected %q", got, "deny") + } +} + +// TestGetConfigReusesParsedConfig checks that an unchanged file is not parsed +// again. The cache keys on modification time and size, so the test rewrites the +// file with different content of the same length and restores the timestamps: +// the previously parsed configuration is expected to survive. This is also the +// documented limitation of the approach. +func TestGetConfigReusesParsedConfig(t *testing.T) { + path := filepath.Join(t.TempDir(), "web-config.yml") + + writeConfigFile(t, path, "http_server_config:\n http2: true\n") + if c := getConfigOrFail(t, path); !c.HTTPConfig.HTTP2 { + t.Fatal("http2 = false, expected true") + } + + info, err := os.Stat(path) + if err != nil { + t.Fatalf("Stat: %v", err) + } + // Same length as the content above. + writeConfigFile(t, path, "http_server_config:\n http2: nope\n") + if err := os.Chtimes(path, info.ModTime(), info.ModTime()); err != nil { + t.Fatalf("Chtimes: %v", err) + } + + c, err := getConfig(path) + if err != nil { + t.Fatalf("getConfig re-parsed a file it should have taken from the cache: %v", err) + } + if !c.HTTPConfig.HTTP2 { + t.Error("http2 = false, expected the cached configuration") + } +} + +// TestGetConfigReturnsACopy checks that a caller cannot change what the next +// caller sees. +func TestGetConfigReturnsACopy(t *testing.T) { + path := filepath.Join(t.TempDir(), "web-config.yml") + writeConfigFile(t, path, "http_server_config:\n http2: true\n") + + first := getConfigOrFail(t, path) + first.HTTPConfig.HTTP2 = false + + if second := getConfigOrFail(t, path); !second.HTTPConfig.HTTP2 { + t.Error("a change to one caller's configuration was visible to the next") + } +} + +// TestGetConfigDoesNotCacheFailures checks that a file which does not parse is +// reported every time, rather than once. +func TestGetConfigDoesNotCacheFailures(t *testing.T) { + path := filepath.Join(t.TempDir(), "web-config.yml") + writeConfigFile(t, path, "this is not a valid configuration\n") + + for i := range 2 { + if _, err := getConfig(path); err == nil { + t.Fatalf("getConfig call %d = nil, expected an error", i+1) + } + } +} + +// TestGetConfigMissingFile checks that a missing file is still an error. +func TestGetConfigMissingFile(t *testing.T) { + if _, err := getConfig(filepath.Join(t.TempDir(), "does-not-exist.yml")); err == nil { + t.Error("getConfig() = nil, expected an error") + } +} + +func BenchmarkGetConfig(b *testing.B) { + path := filepath.Join(b.TempDir(), "web-config.yml") + content := "http_server_config:\n headers:\n X-Frame-Options: deny\n" + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + b.Fatalf("Unable to write config: %v", err) + } + + b.ReportAllocs() + for b.Loop() { + if _, err := getConfig(path); err != nil { + b.Fatal(err) + } + } +} diff --git a/web/tls_config.go b/web/tls_config.go index 022a839e..613b53f2 100644 --- a/web/tls_config.go +++ b/web/tls_config.go @@ -27,6 +27,7 @@ import ( "slices" "strconv" "strings" + "sync" "time" "github.com/coreos/go-systemd/v22/activation" @@ -160,7 +161,64 @@ type RateLimiterConfig struct { Interval time.Duration `yaml:"interval"` } +// configCache memoizes parsed configuration files. The file is consulted on +// every request so that changes are picked up without a restart, and parsing it +// each time is by far the most expensive part of serving one. +// +// An entry is reused only while the file's modification time and size are both +// unchanged, so an edit is picked up on the next request as before. A write +// that leaves both identical is not noticed; on filesystems whose modification +// times have sub-second resolution that needs a rewrite within the same +// nanosecond, and elsewhere within the same second, at exactly the same length. +type configCache struct { + mtx sync.Mutex + entries map[string]configCacheEntry +} + +type configCacheEntry struct { + modTime time.Time + size int64 + config Config +} + +var parsedConfigs = &configCache{entries: make(map[string]configCacheEntry)} + +// get returns the cached configuration for path if it was parsed from a file +// with the same modification time and size. It returns a copy so that a caller +// cannot change what the next one sees. The maps and slices inside are shared, +// and are only ever read. +func (c *configCache) get(path string, info os.FileInfo) *Config { + c.mtx.Lock() + defer c.mtx.Unlock() + + entry, ok := c.entries[path] + if !ok || entry.size != info.Size() || !entry.modTime.Equal(info.ModTime()) { + return nil + } + config := entry.config + return &config +} + +func (c *configCache) set(path string, info os.FileInfo, config Config) { + c.mtx.Lock() + defer c.mtx.Unlock() + + c.entries[path] = configCacheEntry{ + modTime: info.ModTime(), + size: info.Size(), + config: config, + } +} + func getConfig(configPath string) (*Config, error) { + info, err := os.Stat(configPath) + if err != nil { + return nil, err + } + if c := parsedConfigs.get(configPath, info); c != nil { + return c, nil + } + content, err := os.ReadFile(configPath) if err != nil { return nil, err @@ -178,7 +236,14 @@ func getConfig(configPath string) (*Config, error) { err = validateHeaderConfig(c.HTTPConfig.Header) } c.TLSConfig.SetDirectory(filepath.Dir(configPath)) - return c, err + if err != nil { + // A file that does not parse is not cached, so that the error is + // reported again for every request until it is fixed. + return c, err + } + + parsedConfigs.set(configPath, info, *c) + return c, nil } func getTLSConfig(configPath string) (*tls.Config, error) {