Follow-up from "Add Sidekiq daemon memory killer"
The following discussion from !16900 (merged) should be addressed:
-
First place: @ayufan commented on a discussion: (+1 comment) We also have
monotic_time
method somewhere. Yes, we will use it as a follow-up on this MR.
There are several other places we uses Time.now
.
deadline = Time.now + GRACE_BALLOON_SECONDS.seconds
...
break if Time.now > deadline
...
deadline = Time.now + time
sleep(CHECK_INTERVAL_SECONDS) while enabled? && any_jobs? && Time.now < deadline
...
-
Second case: !16900 (comment 218951519)
In both cases, if system DateTime changes, we will get wrong behaviour. Proposed solution:
- Use
monotic_time
in both places - Add extra logic to safe-guard.
- In second case, we should ignore
negative time_elapsed
(return 0), no need formax_time_elapsed
because we already havememory_killer_max_memory_growth_kb
in place. - Also In second case, if we want to make it super-safe, we could have a default value for
memory_killer_max_memory_growth_kb
. Basically change this line:max_memory_growth_kb = get_job_options(job, 'memory_killer_max_memory_growth_kb', MAX_MEMORY_KB).to_i
- In second case, we should ignore
To
# 300M KB is large enough as a DEFAULT value.
# We expect developer/admin always set `memory_killer_max_memory_growth_kb` explicitly
# If developer/admin does not set `memory_killer_max_memory_growth_kb`, there is no reason to allow it exceed more than 300M by default. This is just for extra-safe.
MAX_MEMORY_GROWTH_KB = 500_000
max_memory_growth_kb = get_job_options(job, 'memory_killer_max_memory_growth_kb', MAX_MEMORY_GROWTH_KB).to_i
Edited by 🤖 GitLab Bot 🤖