Dynamic Config #664
Dynamic Config #664
Conversation
There was a problem hiding this comment.
Reads really well, I like how this refreshes fixed values a lot more than having that bloat directly in each instance of Dynamic.
I'd really like to have some Unit tests for the cursed reflect logic, AI should be pretty great at writing that sort of thing.
Also linters are complaining.
deps/config/dynamic.go
Outdated
| } | ||
|
|
||
| func (r *cfgRoot) changeMonitor() { | ||
| lastTimestamp := time.Now().Add(-30 * time.Second) // plenty of time for start-up |
There was a problem hiding this comment.
I'd prefer some global flag for this. Large storage servers with millions of files on hard drives may need a few minutes for proper startup
There was a problem hiding this comment.
A simpler alternative is to look for changes at start-up. A single table read I/O is a small price to pay.
deps/config/dynamic.go
Outdated
| func() { | ||
| DynamicMx.Lock() | ||
| defer DynamicMx.Unlock() | ||
| ApplyLayers(context.Background(), r.treeCopy, configs) |
There was a problem hiding this comment.
Linter somewhat rightfully complains about a missing err check
deps/config/dynamic.go
Outdated
| defer DynamicMx.Unlock() | ||
| ApplyLayers(context.Background(), r.treeCopy, configs) | ||
| }() | ||
| DynamicMx.Lock() |
2 participants
Benefit:
Compatible configs populate the latest value (with respect to the layers) every 30 seconds on running servers without restarting.
Work Remaining:
Decision-Points:
Punchlist (for future PRs):
-- XXMax P4, medium difficulty
-- StorageMarketConfig
--- MK12 - all here, P3, easy-ish
--- IPNI - no need?