[WIP] Remove keepalive timers from Workers #1436
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1436 +/- ##
==========================================
- Coverage 87.52% 87.51% -0.01%
==========================================
Files 105 105
Lines 7918 7931 +13
==========================================
+ Hits 6930 6941 +11
- Misses 988 990 +2
Continue to review full report at Codecov.
|
mudler
force-pushed
the
remove_worker_keepalives
branch
from
ce72658
to
a8ef8f1
Compare
This should decrease the websocket load from the worker side - given the instance number we use it to calculate the timer range to send status messages to the websocket server.
mudler
force-pushed
the
remove_worker_keepalives
branch
from
a8ef8f1
to
b7c4c4d
Compare
| @@ -148,19 +145,20 @@ sub websocket_commands { | |||
| if ($job && $job->{id}) { | |||
| $OpenQA::Worker::Common::job = $job; | |||
| log_debug("Job " . $job->{id} . " scheduled for next cycle"); | |||
| my $send_status = sub { | |||
| log_debug("Sending IMMEDIATELY worker status to $host"); | |||
There was a problem hiding this comment.
I saw that you just moved this debug line but I wanted to ask you anyway what it means. I don't understand the english sentence here. Could you clarify that line?
There was a problem hiding this comment.
Because that message is sent right away when the job starts/stop.
| # Feature scaling - we do not know the instance maximum per worker, we take DEAD_DETECTION_THRESHOLD as max. | ||
| my $imax = $i > DEAD_DETECTION_THRESHOLD ? $i : DEAD_DETECTION_THRESHOLD; | ||
| my $worker_status_timer | ||
| = int(MIN_STATUS_TIMER + ((($i - 1) * (DEAD_DETECTION_THRESHOLD - MIN_STATUS_TIMER)) / ($imax - 1))); |
There was a problem hiding this comment.
IMHO line breaks like these should be avoided so … use a temporary variable assignment?
There was a problem hiding this comment.
those are temporary variables already
There was a problem hiding this comment.
I meant
my $foo = ($i - 1) * (DEAD_DETECTION_THRESHOLD - MIN_STATUS_TIMER);
my $worker_status_timer = int(MIN_STATUS_TIMER + ( $foo / ($imax - 1)));There was a problem hiding this comment.
Right, and that would add to the code just another useless variable for 1 statement forcing even the logic to come up with a variable name, that would be meaningless anyway:
my $foo = ($i - 1) * (DEAD_DETECTION_THRESHOLD - MIN_STATUS_TIMER);
my $bar = ( $foo / ($imax - 1));
my $worker_status_timer = int(MIN_STATUS_TIMER + $bar);How you are going to call $foo and $bar then? $a and $b ? ;-)
At this point, if the line break is the problem, it's better to just shrink the constants names, e.g:
my $status_timer = int(MIN_TIMER + ((($i - 1) * (MAX_TIMER - MIN_TIMER)) / ($imax - 1)));| my $worker_status_timer | ||
| = int(MIN_STATUS_TIMER + ((($i - 1) * (DEAD_DETECTION_THRESHOLD - MIN_STATUS_TIMER)) / ($imax - 1))); | ||
| log_debug("## worker_status timer will be sending status each $worker_status_timer seconds") | ||
| if $verbose; |
There was a problem hiding this comment.
I don't undersand what thas flag is for. Isn't log_debug implicit for "verbose"?
There was a problem hiding this comment.
it's to display the debug message when the client has the optional flag --verbose specified as argument in the command line.
|
@okurz it's a wip, and was discussed already with @coolo to remove (now) duplicated workload on the websocket server - something like that was expected. I'll put a complete description when i'm sure about all the required changes. Edit: i forgot to mention, that the problem was already discussed in #1432 and on #1433 as well, this is a follow-up discussion to identify possible fixes. The issues are related. |
|
Closing, superseded by #1486 |
No description provided.