perf: add TTL-based caching for template file stat checks #458
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds TTL-based caching for os.Stat() calls on template files to reduce syscall overhead. When a cached template is accessed within the configured TTL window, the stat check is skipped, assuming the file hasn't changed. This optimization is particularly beneficial in high-frequency template rendering scenarios.
Changes:
- Added
statTimefield to track when file stats were last verified, with configurable TTL (default 1s) - Introduced
--stat-cache-ttlCLI flag andstat_cache_ttlTOML option for configuration - Added
confd_template_stat_cache_hits_totalmetric to monitor stat cache effectiveness
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/template/template_cache.go | Implements stat caching logic with TTL checks and statTime tracking |
| pkg/template/template_cache_test.go | Updates all test initialization calls and adds comprehensive TTL behavior tests |
| pkg/metrics/metrics.go | Adds new counter metric for tracking stat cache hits |
| cmd/confd/config.go | Adds TOML configuration parsing for stat_cache_ttl |
| cmd/confd/cli.go | Adds CLI flag and default constant for stat cache TTL |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| if enabled { | ||
| log.Debug("Template cache enabled") | ||
| log.Debug("Template cache enabled with stat TTL %v", statTTL) |
There was a problem hiding this comment.
The log.Debug call is missing a format specifier. It should use string formatting (e.g., log.Debugf) or concatenate the value properly to avoid treating statTTL as a format argument.
| log.Debug("Template cache enabled with stat TTL %v", statTTL) | |
| log.Debugf("Template cache enabled with stat TTL %v", statTTL) |
| if d, err := time.ParseDuration(tomlCfg.StatCacheTTL); err == nil { | ||
| if cli.StatCacheTTL == DefaultStatCacheTTL { | ||
| cli.StatCacheTTL = d | ||
| } |
There was a problem hiding this comment.
When time.ParseDuration fails, the error is silently ignored. Users won't know their configuration is invalid. Consider logging a warning or returning an error when the stat_cache_ttl value cannot be parsed.
| if d, err := time.ParseDuration(tomlCfg.StatCacheTTL); err == nil { | |
| if cli.StatCacheTTL == DefaultStatCacheTTL { | |
| cli.StatCacheTTL = d | |
| } | |
| d, err := time.ParseDuration(tomlCfg.StatCacheTTL) | |
| if err != nil { | |
| log.Debug("Invalid stat_cache_ttl %q in config file %s: %v; using default %s", tomlCfg.StatCacheTTL, cli.ConfigFile, err, DefaultStatCacheTTL) | |
| } else if cli.StatCacheTTL == DefaultStatCacheTTL { | |
| cli.StatCacheTTL = d |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #458 +/- ##
==========================================
+ Coverage 59.85% 60.00% +0.14%
==========================================
Files 48 48
Lines 5244 5273 +29
==========================================
+ Hits 3139 3164 +25
- Misses 1899 1901 +2
- Partials 206 208 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Add configurable TTL for caching os.Stat() results in the template cache. This reduces syscall overhead when templates are processed frequently, such as during batch processing or rapid watch mode updates. - Add statTime field to cachedTemplate to track last stat verification - Add statTTL to TemplateCache (default: 1s, 0 disables) - Add --stat-cache-ttl CLI flag and stat_cache_ttl TOML config option - Add confd_template_stat_cache_hits_total metric to track skipped stats - Update InitTemplateCache signature to accept statTTL parameter Closes #446
abtreece
force-pushed
the
perf/stat-cache-ttl
branch
from
8e6591b to
72b823c
Compare
1 participant
Summary
os.Stat()results in the template cache to reduce syscall overhead--stat-cache-ttlCLI flag (default: 1s) orstat_cache_ttlTOML optionconfd_template_stat_cache_hits_totaltracks how often stat syscalls are skippedTest plan
go test ./pkg/template/... -run TestStatCacheTTL -vmake build./bin/confd --help | grep stat-cache-ttlCloses #446