Enforce memory-watchdog by default
What does this MR do and why?
We recently rolled out a new memory killer, memory-watchdog, to replace and unify existing memory killers. As of 15.6 this was also enabled by default for Omnibus users (instead of Puma Worker Killer, the previous memory killer this replaced), but since its primary functionality was guarded by two different ops feature toggles that were only enabled on SaaS, it was not running by default for those users.
The two feature flags are:
-
gitlab_memory_watchdog
: this was a general guard that stopped the watchdog from doing anything that we used to safely roll this out. I removed this since I don't think we need it anymore. -
enforce_memory_watchdog
: this is an ops toggle that "defuses" the watchdog to only log violations, but not enforce worker restarts. This is still useful to have and I merely changed this to be enabled by default.
Together these changes should ensure that the watchdog curbs memory use by default.
Screenshots or screen recordings
Screenshots are required for UI changes, and strongly recommended for all other merge requests.
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Related to #386977 (closed)
Merge request reports
Activity
changed milestone to %15.8
assigned to @mkaeppler
mentioned in issue #386977 (closed)
- A deleted user
added backend feature flag labels
- Resolved by Matthias Käppler
1 Warning Please add a merge request subtype to this merge request. Reviewer roulette
Changes that require review have been detected!
Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:
Category Reviewer Maintainer backend Bojan Marjanović (
@bmarjanovic
) (UTC+1, same timezone as@mkaeppler
)Thong Kuah (
@tkuah
) (UTC+13, 12 hours ahead of@mkaeppler
)To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.
If needed, you can retry the
danger-review
job that generated this comment.Generated by
DangerAllure report
allure-report-publisher
generated test report!e2e-package-and-test:
test report for 0dea852dexpand test summary
+-----------------------------------------------------------------------+ | suites summary | +------------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +------------------+--------+--------+---------+-------+-------+--------+ | Create | 972 | 11 | 32 | 0 | 1015 | ❌ | | Plan | 371 | 0 | 0 | 0 | 371 | ✅ | | ModelOps | 0 | 0 | 6 | 0 | 6 | ➖ | | Manage | 386 | 4 | 18 | 6 | 408 | ❌ | | Secure | 42 | 0 | 6 | 0 | 48 | ✅ | | Fulfillment | 12 | 0 | 84 | 0 | 96 | ✅ | | Govern | 256 | 0 | 6 | 0 | 262 | ✅ | | Analytics | 12 | 0 | 0 | 0 | 12 | ✅ | | Verify | 298 | 0 | 20 | 6 | 318 | ❗ | | Configure | 0 | 0 | 18 | 0 | 18 | ➖ | | Release | 36 | 0 | 0 | 0 | 36 | ✅ | | Package | 125 | 0 | 22 | 12 | 147 | ❗ | | Framework sanity | 0 | 0 | 8 | 0 | 8 | ➖ | | Monitor | 24 | 0 | 0 | 0 | 24 | ✅ | | Systems | 2 | 0 | 0 | 0 | 2 | ✅ | | Data Stores | 11 | 0 | 3 | 0 | 14 | ✅ | +------------------+--------+--------+---------+-------+-------+--------+ | Total | 2547 | 15 | 223 | 24 | 2785 | ❌ | +------------------+--------+--------+---------+-------+-------+--------+
assigned to @nmilojevic1 and unassigned @mkaeppler
@sdobrovolschii can you please do the initial review?
requested review from @sdobrovolschii
- Resolved by Aleksei Lipniagov
Hi @nmilojevic1
MR looks good, the only thing I was thinking about, was why not enablinggitlab_memory_watchdog
by default first (default_enabled: true
), before removing it. Wouldn't it make things a bit safer?
@sdobrovolschii
, thanks for approving this merge request.This is the first time the merge request is approved. To ensure full test coverage, a new pipeline will be started shortly.
For more info, please refer to the following links:
added pipeline:mr-approved label
removed review request for @sdobrovolschii
- Resolved by Aleksei Lipniagov
@alipniagov can you please do the maintainer review?
requested review from @alipniagov
enabled an automatic merge when the pipeline for efa5519c succeeds
mentioned in commit 3e8a6c95
added workflowstaging-canary label and removed workflowin dev label
added workflowcanary label and removed workflowstaging-canary label
added workflowstaging label and removed workflowcanary label
added workflowproduction label and removed workflowstaging label
added Pick into 15.7 label
added severity3 label
added bugperformance label
picked the changes into the branch
15-7-stable-ee-patch-4
with commit 6f7fcb0fAutomatically picked into !108679 (merged), will merge into
15-7-stable-ee
ready for15.7.3-ee
.Edited by Ahmad Tolbamentioned in commit 6f7fcb0f
removed Pick into 15.7 label
mentioned in merge request !108679 (merged)
added workflowpost-deploy-db-staging label and removed workflowproduction label
added workflowpost-deploy-db-production label and removed workflowpost-deploy-db-staging label
mentioned in merge request !108595 (closed)
added releasedcandidate label
mentioned in merge request kubitus-project/kubitus-installer!1829 (merged)
mentioned in issue #393754 (closed)
added releasedpublished label and removed releasedcandidate label