pkg/reloader: improve detection of directory changes #3136
Conversation
simonpasquier
force-pushed
the
improve-reloader-for-dirs
branch
from
d4f6e6f
to
434e274
Compare
|
@simonpasquier will this have any effect on falky reloader tests #2622? Also e2e tests are failing. |
|
Kemal, thanks for pointing the issue, I'll look into it. |
simonpasquier
force-pushed
the
improve-reloader-for-dirs
branch
2 times, most recently
from
48a0ad0
to
ac8ecd2
Compare
When watching for changes in directories, the reloader used to rely only on the watch interval and not the inotify events. This commit implements a more efficient detection of changes for watched directories. The change also adds a new `DelayInterval` option that allows to delay the config reload after no additional event are received. Finally a new metric, `thanos_sidecar_reloader_config_apply_operations_total`, is added and `thanos_sidecar_reloader_config_apply_errors_total` has been renamed to `thanos_sidecar_reloader_config_apply_operations_failed_total` for consistency. Signed-off-by: Simon Pasquier <spasquie@redhat.com>
simonpasquier
force-pushed
the
improve-reloader-for-dirs
branch
from
ac8ecd2
to
1ba6383
Compare
|
Let's have another pair of eyes before we merge this. cc @thanos-io/thanos-maintainers |
Signed-off-by: Simon Pasquier <spasquie@redhat.com>
|
prometheus-operator/prometheus-operator#3457 is the prometheus-operator PR including this change. I'm not sure that this PR will help to solve #2622. @kakkoyun do you have a recent CI failure for this? I've run the unit tests several times (>100) on my machine with |
|
Thanks a lot for addressing issues @simonpasquier 💜 The flakiness happens on a single core and relatively slow CPU setups (such as CI). It's ok, it's not our priority for this PR. Let's see if it helps after merging this. |
Changes
When watching for changes in directories, the reloader used to rely only
on the watch interval and not on the inotify events. This commit implements
a more efficient detection of changes for watched directories.
The change also adds a new
DelayIntervaloption that allows to delaythe config reload after no additional event are received.
Finally a new metric,
thanos_sidecar_reloader_config_apply_operations_total, is added andthanos_sidecar_reloader_config_apply_errors_totalhas been renamedto
thanos_sidecar_reloader_config_apply_operations_failed_totalforconsistency.
Verification
I've modified the unit test to validate that the reloader reacts on file changes (without the periodic watch). I've added a unit test to validate that the reloader's periodic watch detects a change in case it hasn't been caught by fsnotify.
I've also included this PR in prometheus-operator/prometheus-operator#3457 which is the change that triggered this PR.