From 6f2be48cc9a34ac1172f7e6b5f4bbc41adbb3b4d 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 --- docs/web-configuration.md | 6 +- web/config_cache_test.go | 137 ++++++++++++++++++++++++++++++++++++++ web/tls_config.go | 68 ++++++++++++++++++- 3 files changed, 208 insertions(+), 3 deletions(-) create mode 100644 web/config_cache_test.go diff --git a/docs/web-configuration.md b/docs/web-configuration.md index c3b1cf12..575c5db1 100644 --- a/docs/web-configuration.md +++ b/docs/web-configuration.md @@ -11,8 +11,10 @@ defined by the scheme described below. Brackets indicate that a parameter is optional. For non-list parameters the value is set to the specified default. -The file is read upon every http request, such as any change in the -configuration, so the certificates are picked up immediately. +The file is consulted upon every http request, so any change in the +configuration, such as the certificates, is picked up immediately. It is only +re-read and re-parsed when its modification time or size has changed since the +previous request. Generic placeholders are defined as follows: 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 b40be6bb..69d755c5 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" @@ -147,7 +148,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. +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 + } + return entry.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 { + // Hand out 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. + cached := *c + return &cached, nil + } + content, err := os.ReadFile(configPath) if err != nil { return nil, err @@ -165,7 +223,15 @@ 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) + cached := *c + return &cached, nil } func getTLSConfig(configPath string) (*tls.Config, error) {