Skip to content
Snippets Groups Projects

Enforce memory-watchdog by default

Merged Matthias Käppler requested to merge 386977-default-enable-watchdog into master
All threads resolved!

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.

Related to #386977 (closed)

Merge request reports

Merged results pipeline #743378577 passed with warnings

Pipeline: GitLab

#743391450

    Pipeline: GitLab

    #743391514

      Merged results pipeline passed with warnings for efa5519c

      Test coverage 81.48% (13.82%) from 2 jobs
      Approved by

      Merged by Aleksei LipniagovAleksei Lipniagov 2 years ago (Jan 10, 2023 6:48pm UTC)

      Pipeline #743473128 passed

      Pipeline passed for 3e8a6c95 on master

      Test coverage 67.88% (13.82%) from 2 jobs
      10 environments impacted.

      Activity

      Filter activity
      • Approvals
      • Assignees & reviewers
      • Comments (from bots)
      • Comments (from users)
      • Commits & branches
      • Edits
      • Labels
      • Lock status
      • Mentions
      • Merge request status
      • Tracking
    • 1 Warning
      :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ć current availability (@bmarjanovic) (UTC+1, same timezone as @mkaeppler) Thong Kuah current availability (@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 :repeat: danger-review job that generated this comment.

      Generated by :no_entry_sign: Danger

    • Matthias Käppler resolved all threads

      resolved all threads

    • Allure report

      allure-report-publisher generated test report!

      e2e-package-and-test: :x: test report for 0dea852d

      expand 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

    • Stanislav Dobrovolschii approved this merge request

      approved this merge request

    • :wave: @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:

    • removed review request for @sdobrovolschii

    • requested review from @alipniagov

    • Aleksei Lipniagov approved this merge request

      approved this merge request

    • Aleksei Lipniagov resolved all threads

      resolved all threads

    • Aleksei Lipniagov enabled an automatic merge when the pipeline for efa5519c succeeds

      enabled an automatic merge when the pipeline for efa5519c succeeds

    • mentioned in commit 3e8a6c95

    • added workflowstaging label and removed workflowcanary label

    • GitLab Release Tools Bot picked the changes into the branch 15-7-stable-ee-patch-4 with commit 6f7fcb0f

      picked the changes into the branch 15-7-stable-ee-patch-4 with commit 6f7fcb0f

    • Automatically picked into !108679 (merged), will merge into 15-7-stable-ee ready for 15.7.3-ee.

      Edited by Ahmad Tolba
    • mentioned in commit 6f7fcb0f

    • mentioned in merge request !108679 (merged)

    • Nikola Milojevic mentioned in merge request !108595 (closed)

      mentioned in merge request !108595 (closed)

    • mentioned in issue #393754 (closed)

    • Please register or sign in to reply
      Loading