Skip to content

config-reloader: trigger reload after initial directory sync - #2528

Merged
vrutkovs merged 6 commits into
VictoriaMetrics:masterfrom
shunki-fujita:config-reloader-initial-sync-reload
Aug 31, 2026
Merged

config-reloader: trigger reload after initial directory sync#2528
vrutkovs merged 6 commits into
VictoriaMetrics:masterfrom
shunki-fujita:config-reloader-initial-sync-reload

Conversation

@shunki-fujita

@shunki-fujita shunki-fujita commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Fixes #2527

Problem

Since v0.72.0, VMAlert reads its rule files from an emptyDir populated by the config-init init container and synced by the config-reloader sidecar. If the rules ConfigMap is updated between the init container's copy and the sidecar starting, the sidecar's initial sync writes the new content to the target dir but never notifies the application. The application keeps serving the snapshot it read at startup, and if the ConfigMap never changes again, no fsnotify event ever fires — the stale state is permanent.

Fix

  • dirWatcher.start() sends one reload signal after the initial directory sync, provided at least one watched directory completed it. This also applies to watch-only directories (no --target-dir): the same startup race exists there — a change landing between the application's first read and the watch being established produces no event — just with a smaller window.
  • Failed reload calls are retried with exponential backoff (1s, doubling up to 30s) until they succeed. Without this, the startup reload could be lost when the application's reload endpoint is not listening yet, reintroducing the same permanent staleness. This also covers transient failures of event-driven reloads.
  • The directory content hash is committed to the cache only after a successful sync (suggested by @vrutkovs). A failed sync — at startup or on an event — leaves the hash uncommitted, so the next fsnotify event retries it naturally, without a dedicated retry loop. This also fixes a pre-existing gap where a failed event-driven sync was never retried, because the hash had already been cached.

Tests

  • reload retry: the reload call fails twice and succeeds on the third attempt
  • failed initial sync: no startup reload fires, and the next event retries the sync and triggers the reload even for unchanged content, since the hash was never committed
  • partial failure: a failing pair does not hold back the startup reload for a pair that synced successfully
  • liveness: events for healthy directories are processed while another pair's sync keeps failing
  • no watched dirs: no startup reload
  • existing dir-watcher tests drain the startup signal, pinning the new behavior

@shunki-fujita
shunki-fujita marked this pull request as draft August 21, 2026 06:36

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Re-trigger cubic

@shunki-fujita
shunki-fujita force-pushed the config-reloader-initial-sync-reload branch 2 times, most recently from 623729e to e7629bb Compare August 21, 2026 07:02
@shunki-fujita
shunki-fujita marked this pull request as ready for review August 21, 2026 07:06

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 4 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread cmd/config-reloader/file_watch.go Outdated
@vrutkovs vrutkovs self-assigned this Aug 21, 2026
Comment thread cmd/config-reloader/file_watch.go Outdated
@shunki-fujita
shunki-fujita requested a review from vrutkovs August 26, 2026 07:55

@vrutkovs vrutkovs left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs CHANGELOG entry too

Comment thread cmd/config-reloader/file_watch.go Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 3 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic

Comment thread cmd/config-reloader/file_watch.go
@shunki-fujita
shunki-fujita requested a review from vrutkovs August 28, 2026 08:15
@vrutkovs
vrutkovs force-pushed the config-reloader-initial-sync-reload branch from 66ee787 to 6aab738 Compare August 31, 2026 06:54

@vrutkovs vrutkovs left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution!

@vrutkovs
vrutkovs enabled auto-merge (squash) August 31, 2026 06:55
@vrutkovs
vrutkovs force-pushed the config-reloader-initial-sync-reload branch from 6aab738 to 1c5b6d7 Compare August 31, 2026 07:34
Instead of caching the directory hash inside updateCache before the copy
happens, return the hash and store it only once the sync succeeded. A failed
sync leaves the hash uncommitted, so the next fsnotify event retries it
naturally. This removes the dedicated initial-sync retry loop.

Suggested by @vrutkovs in review.
Reloading before any directory was mirrored just re-reads the old content.
Trigger the startup reload only when at least one watched pair (including
watch-only dirs) completed its initial sync.
@vrutkovs
vrutkovs force-pushed the config-reloader-initial-sync-reload branch from 1c5b6d7 to 938f5da Compare August 31, 2026 07:34
@vrutkovs
vrutkovs merged commit 0a53d7d into VictoriaMetrics:master Aug 31, 2026
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vmalert: rules stay stale forever when the rules ConfigMap is updated during pod startup

2 participants