Fix incomplete Windows support of MultiSpawnServer

ashie commented 4 years ago

This commit intents to fix the following issues of Fluentd:

https://github.com/fluent/fluentd/issues/3066: Unexpected error Broken pipe when trying to shutdown fluentd with multiple workers
https://github.com/fluent/fluentd/pull/3131: Add win32 events alternative to unix signals

Current implementation of MultiSpawnServer has following issues. This PR fixes them:

multiple workers feature isn't supported correctly on Windows
graceful stop & restart aren't supported on Windows
reload commands isn't supported on Windows

~~In addition, this PR also fixes default behavior of stop and restart methods of MultiSpawnServer. They do nothing by default because these methods expect that auto_tick is enabled although MultiSpawnServer disables it by default. When auto_tick is disabled, users of this class will expect that it sends a signal (or command) immediately. (If you don't want to change this behavior, I'll remove this commit.)~~

repeatedly commented 4 years ago

Test failed. Could you check it?

ashie commented 4 years ago

In addition, this PR also fixes default behavior of stop and restart methods of MultiSpawnServer. They do nothing by default because these methods expect that auto_tick is enabled although MultiSpawnServer disables it by default. When auto_tick is disabled, users of this class will expect that it sends a signal (or command) immediately.

I misunderstood it. tick method of ProccessManager is called in run method of MultiWorkerServer. Although tick isn't called on Windows, it's caused by another reason. It's because hearbeat feature isn't implemented on Windows: https://github.com/treasure-data/serverengine/blob/master/lib/serverengine/process_manager.rb#L250

I'll remove this commit so that failed tests will be fixed. To implement graceful stop for Windows instead, I'll add another fix.

ashie commented 4 years ago

I'll remove this commit so that failed tests will be fixed. To implement graceful stop for Windows instead, I'll add another fix.

I've pushed the fix.

repeatedly commented 4 years ago

Thanks!

ashie commented 4 years ago

Thanks! BTW there are few tests for MultiSpawServer, I'm considering add more tests.

treasure-data / serverengine

Fix incomplete Windows support of MultiSpawnServer #105