Block excessive gracefulReload requests

[kenhys]

Which issue(s) this PR fixes:

Fixes #3341

What this PR does / why we need it:

In the previous versions, /api/config.gracefulReload call doesn't
restrict excessive API calls. It causes the following error when
already gracefulReload is executing.

Worker 0 finished unexpectedly with signal SIGKILL

This commit mitigates such a situation by limit a API call.
(it gives an interval in 60 seconds by default)

Docs Changes:

https://docs.fluentd.org/deployment/system-config
@blocking_reload_interval should be added in another PR.

Release Note:

N/A

NOTE: Ideally it should wait and detects graceful reload finish, but
there is no easy way to synchronize internal state between
ServerModule(RPC::Server#mount_proc) and WorkerModule (reload_config).

[kenhys]

rerun windows CI again.

[kenhys]

TODO: @system_config.blocking_reload_interval returns default value instead of customized one. 🤔

[kenhys]

This PR should be ready for v1.13.1? or later release. (not for v1.13.0)

[cosmo0920]

@kenhys All of tests which are running on GitHub Actions is failed. Could you check them?

[kenhys]

The problem of the current approach to customize blocking_read_interval:

• config[:blocking_read_interval] is nil in Fluent::ServerModule#before_run

• because of the above, @block_reload_until is not overriden in Fluent::ServerModule#run

[kenhys]

Broken test cases were fixed.

[kenhys]

Rebased with recent master.

[kenhys]

Removed if @blocking_reload_interval check, and tweak test case a bit.

[kenhys]

Changed to use @blocking_reload_interval instead of config[:blocking_reload_interval] and removed a redundant conditional assignment.

[ashie]

It can block excessive RPC call but can't block excessive SIGUSR2 call.
Could you consider also SIGUSR2 case?

[kenhys]

I'll fix it, too.

[kenhys]

SIGUSR2 is also supported.

[ashie]

CI failed on Windows:

2021-06-07T05:42:25.0561362Z Error: test_blocking_signal_handler(SupervisorTest::gracefulReload): NoMethodError: undefined method `stop_rpc_server' for nil:NilClass
2021-06-07T05:42:25.0565425Z D:/a/fluentd/fluentd/test/test_supervisor.rb:299:in `teardown'

[Cryptophobia]

When gracefulReload is triggered, a new thread for reloading is created at both supervisor process and worker process.

If the threads run sequentially, would that be enough to keep consistency, no matter how many times gracefulReload is called? This would the similar as implementing a queue for the threads?

I have no opinions against trying to solve this as long as there is a flag to keep current behavior the same because in projects like KFO, we do not wait or check the status for a return from gracefulReload endpoint when issuing multiple requests sequentially. We hope that fluentd will be able to handle this logic of what to do, in situations when configuration has changed in a short amount of time.

The other important consideration is, if a gracefulReload worker thread fails, how do we make sure that unfinished configuration change is still gracefullyReloaded? If it only fails once and blocks, the subsequent configuration change will not be able to gracefulReload, and that would be bad design, because then subsequent calls to an API (gracefulReload in this case) depend on the processing of previous API calls. Should the API not be independent and the processing of the config reloading need to be idempotent?

By idempotent, I just mean if we send 2 calls to gracefulReload, fluentd should just finish processing both sequentially. Any blocking would change behavior of the API and configuration changes could be missed, also API calls ignored.

[kenhys]

Thanks @alex-vmw, @Cryptophobia, @ashie
Current approach makes it problematic, I withdraw PR.