fix: ensure sleep KILL_OLD_DELAY #45
Conversation
| @@ -389,7 +389,9 @@ sub start_server { | |||
| my $kill_old_delay = defined $ENV{KILL_OLD_DELAY} ? $ENV{KILL_OLD_DELAY} : $ENV{ENABLE_AUTO_RESTART} ? 5 : 0; | |||
| if ($kill_old_delay != 0) { | |||
| print STDERR "sleeping $kill_old_delay secs before killing old workers\n"; | |||
| sleep $kill_old_delay; | |||
| while ($kill_old_delay > 0) { | |||
| $kill_old_delay -= sleep $kill_old_delay; | |||
There was a problem hiding this comment.
Could you adjust the code so that $kill_old_delay will be decremented by one in case sleep returns zero?
My understanding is that the seconds returned by sleep is rounded down. And my fear that the loop might run forever if signals get delivered more than once a second.
There was a problem hiding this comment.
I might be better, IMHO, to let $kill_old_delay be defined as the "lower bound" of sleep time to ensure new workers get ready to work. And in majority of cases, sleep should not be interrupted by continuous signals with interval < 1s.
However, I agree, it would be much safer to let it be the "upper bound". To avoid edge cases, I will round up the sleep time to at least one second. :)
|
Round it up to 1 second when slept less than 1 second. |
|
Thank you for the fix! Will ship a new version later. |
|
Thanks! |
Changelog diff is:
diff --git a/Changes b/Changes
index c148672..8e955bd 100644
--- a/Changes
+++ b/Changes
@@ -2,6 +2,11 @@ Revision history for Perl extension Server::Starter.
{{$NEXT}}
+0.33 2016-12-13T00:37:49Z
+ - do not kill old worker too early (thanks to Xuanzhong Wei) #45
+ - allow use of `--enable-auto-restart` without an argument (thanks to Slaven Rezic) #41
+
+
0.32 2015-08-25T02:09:18Z
- fix compatibility issue on Solaris (thanks to Syohei YOSHIDA) #40
Changelog diff is:
diff --git a/Changes b/Changes
index 8e955bd..746bfaa 100644
--- a/Changes
+++ b/Changes
@@ -2,11 +2,14 @@ Revision history for Perl extension Server::Starter.
{{$NEXT}}
+0.34 2018-02-26T06:32:16Z
+ - run start_server even if no port (or path) is specified (thanks to Ichito Nagata) #49
+ - add `.` in @inc (thanks to Petr Písař) #47
+
0.33 2016-12-13T00:37:49Z
- do not kill old worker too early (thanks to Xuanzhong Wei) #45
- allow use of `--enable-auto-restart` without an argument (thanks to Slaven Rezic) #41
-
0.32 2015-08-25T02:09:18Z
- fix compatibility issue on Solaris (thanks to Syohei YOSHIDA) #40
@kazuho
As we discussed in #44, I have tried to fix the sleep in
KILL_OLD_DELAYinterrupted by a signal right after the previous one. The old workers will now keep alive before new workers get ready.Please take a look.
I would appreciate it if you could change the minor version number for this fix.
Thanks.