Feat-conf hot-loading capacity #2992
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2992 +/- ##
===========================================
- Coverage 46.76% 39.83% -6.94%
===========================================
Files 295 457 +162
Lines 17172 38992 +21820
===========================================
+ Hits 8031 15533 +7502
- Misses 8287 22182 +13895
- Partials 854 1277 +423 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
todo ：Limit critical configuration hot updates |
There was a problem hiding this comment.
Pull Request Overview
This PR implements hot-loading capacity for configuration files, allowing the application to automatically reload configuration changes without requiring a restart. The implementation adds file system watching capabilities to detect configuration file modifications and triggers hot reloads when changes occur.
- Adds file system watcher using fsnotify to monitor configuration file changes
- Implements hot reload functionality that updates instanceOptions and logger level dynamically
- Introduces graceful shutdown mechanism for the file watcher with proper cleanup
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| once sync.Once | ||
| stopCh = make(chan struct{}) | ||
| watcherWg sync.WaitGroup |
There was a problem hiding this comment.
Global variables for concurrency control create potential race conditions. The stopCh channel and watcherWg should be encapsulated within a struct or managed differently to avoid shared mutable state across goroutines.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
yeah, can not agree with you more.
There was a problem hiding this comment.
yeah, can not agree with you more.
Thanks for the suggestion—given our single watcher and Once/stopOnce guards, this is safe; we’ll consider encapsulation if we later support multiple or restartable watchers
| // Any part of the application that accesses instanceOptions directly will now use the new values. | ||
| instanceOptions = newOpts |
There was a problem hiding this comment.
Direct assignment to the global instanceOptions variable creates a race condition. Multiple goroutines could be reading instanceOptions while it's being updated, leading to inconsistent state. Consider using atomic operations or proper synchronization mechanisms.
Copilot uses AI. Check for mistakes.
loader.go
Outdated
| func StopFileWatcher() { | ||
| logger.Info("Stopping file watcher...") | ||
| close(stopCh) | ||
| watcherWg.Wait() | ||
| logger.Info("File watcher stopped successfully") | ||
| } |
There was a problem hiding this comment.
Closing stopCh multiple times will cause a panic. The function should check if the channel is already closed or use a sync.Once to ensure it's only closed once.
Copilot uses AI. Check for mistakes.
| @@ -64,9 +72,81 @@ func Load(opts ...LoaderConfOption) error { | |||
| } | |||
|
|
|||
| instance := &Instance{insOpts: instanceOptions} | |||
| // start the file watcher | |||
| once.Do(func() { | |||
There was a problem hiding this comment.
The watcherWg.Add() should be called before starting the goroutine to avoid race conditions where StopFileWatcher() could call Wait() before the goroutine increments the counter.
| once.Do(func() { | |
| once.Do(func() { | |
| watcherWg.Add(1) |
Copilot uses AI. Check for mistakes.
|
done |
Feat-conf hot-loading capacity
Fix ：#2925