Add means to bump Markdown cache gradually

Asherah Connorrequested to merge
push-stowknuonxny into master

What does this MR do and why?

Closes Bumping CACHE_COMMONMARK_VERSION is risky (#597379 - closed) by adding a vetted and repeatable method to bump CACHE_COMMONMARK_VERSION, controlled by a pair of version constants (instead of our current one) and a new dynamic FF type to manage the rollout.

  1. Land MR declaring the cache version we're rolling forward to — no behavioural change yet, but the FF for that version is now being checked.
  2. Adjust the FF to control the percentage of cache version checks report the new version, ramping up to 100% gradually while watching database load. This process can take as long as it needs — hours, days, weeks.
  3. Land MR declaring the roll-forward complete; the FF is no longer consulted.

Markdown cache updates currently happen in the following circumstances:

  • The cache column is empty for whatever reason (e.g. not yet generated).
  • The source column for the Markdown has changed.
  • The cached_markdown_version column contains a value less than the current declared cache version.

Note that while I use "cache column" and "source column" in the singular, there can be multiple Markdown columns per record, so this may apply to multiple. There is only one cached_markdown_version per record, however, so bumping that forces a freshening of all caches in that record.

This MR updates the behaviour of the cached_markdown_version column check. The "declared cache version" now depends on an FF read (when the "rolling forward" version is declared). It's OK that a single row may be read sometimes at the current and sometimes at the later version; we do not regress the version when the record's cache version is newer than the current one.

We always write the latest cache version regardless of the cache version check result, as the new write is by definition current.

On using a percentage_of_time FF

Please read the extensively-updated-in-this-MR "Banzai pipeline and parsing" docs regarding the FF type selection. It is by design, and does not trip the concerns that led to its being marked as deprecated (Percentage-based Feature Flags should return th... (#425202 - closed), 2023-09-14: Issues and comments not loading cor... (gitlab-com/gl-infra/production#16366 - closed)). We explicitly do not want the flipper to return the same value for multiple calls in the one request.

References

How to set up and validate locally

Here we walk through two successive cache bumps on our GDKs, using Banzai::Filter::ColorFilter (which renders `#123456`) as a colour chip, like this: #123456) as a visible difference between cached renders. The only steps we skip in the rollout process (as documented in the MR) are the bits where we commit and make MRs to bookmark the process!

We want to confirm the following:

  1. Setting CACHE_COMMONMARK_VERSION_PREVIOUS plus enabling the FF at a percentage causes some but not all visible items to be re-rendered each page load, and that subsequent refreshes get us to(ward) 100%.
  2. Setting the FF to 100% and reloading leaves nothing re-rendered.
  3. "Completing" the rollout (i.e. setting CACHE_COMMONMARK_VERSION_PREVIOUS back to nil) returns the system to its steady state (without any changes in behaviour).
  4. The whole process repeats cleanly.

Prepare: work item with several colour-chip notes

In gdk rails console, seed 10 notes on some issue as (I've picked Issue.first) as root:

user = User.first
issue = Issue.first
10.times do |i|
  colour = format('#%06x', rand(0x1000000))
  Notes::CreateService.new(
    issue.project, user,
    noteable: issue, note: "Colour note #{i + 1}: `#{colour}`"
  ).execute
end
puts Rails.application.routes.url_helpers.project_work_item_url(issue.project, iid: issue.iid)

Open the URL it prints; here's what I see:

image

These are cached at the current CACHE_COMMONMARK_VERSION. Let's confirm all notes are at that version in the Rails console:

[23] pry(main)> issue.notes.pluck(:cached_markdown_version).tally
=> {2162688=>49}
[24] pry(main)> Gitlab::MarkdownCache::CACHE_COMMONMARK_VERSION_SHIFTED
=> 2162688

Looks good.

Bump #1: roll forward by removing ColorFilter

We simulate landing MR1 of a real rollout: a change has been made to the Markdown pipeline, CACHE_COMMONMARK_VERSION_PREVIOUS is set to point at the old version, but the FF for the new version is unset so no existing content re-renders yet.

  1. Edit lib/banzai/pipeline/gfm_pipeline.rb and comment out Filter::ColorFilter in the filters list (line 32 for me right now). This is the change we want to roll out.

  2. Edit lib/gitlab/markdown_cache.rb:

    • Bump CACHE_COMMONMARK_VERSION from 33 to 34.
    • Set CACHE_COMMONMARK_VERSION_PREVIOUS = 33.

  3. Reload the issue page in the browser. All colour chips should still show — reads are rolling "previous" 100% of the time.

  4. In the Rails console, confirm a representative note still reads at the old version:

    [27] pry(main)> Issue.first.notes.last.reload.cached_markdown_version
=> 2162688

Ramp: 10%

  1. Enable the FF for rolling forward to 34 at 10%:

    [2] pry(main)> Feature.enable_percentage_of_time(:markdown_cache_stochastic_rollout_34, 10)
=> true

    Locally, this is the equivalent of the chatops --random invocation in the rollout doc.

  2. Reload the issue page. Some but not all colour chips should vanish; those notes have been rolled "current", re-rendered without ColorFilter, and their cache rewritten at the new version. The rest still show their chip 🍟

    Note that the number that do update is going to be higher than 10% the first time: a single visit causes each note's body to be accessed multiple times in practice. After that, it depends on how many already-updated ones roll higher vs not-yet-updated ones. Here's what I get:

    image

  3. Reload again. More are gone:

    image

  4. Confirm the cache version has progressed (I checked between/during each refresh):

    [4] pry(main)> Issue.first.notes.pluck(:cached_markdown_version).tally
=> {2228224=>37, 2162688=>12}
[5] pry(main)> Issue.first.notes.pluck(:cached_markdown_version).tally
=> {2228224=>39, 2162688=>10}
[6] pry(main)> Issue.first.notes.pluck(:cached_markdown_version).tally
=> {2228224=>43, 2162688=>6}
[7] pry(main)> Issue.first.notes.pluck(:cached_markdown_version).tally
=> {2228224=>47, 2162688=>2}

Ramp: 100%

  1. Set the FF to 100%:

    [13] pry(main)> Feature.enable_percentage_of_time(:markdown_cache_stochastic_rollout_34, 100)
true

  2. Reload once. No colour chips remain, if any did before.

    [14] pry(main)> Issue.first.notes.pluck(:cached_markdown_version).tally
=> {2228224=>49}

Finalise: complete the rollout (MR2)

Now we pretend to land MR2: the rollout window closes, and the system returns to steady state.

  1. Edit lib/gitlab/markdown_cache.rb and set CACHE_COMMONMARK_VERSION_PREVIOUS = nil. Leave CACHE_COMMONMARK_VERSION = 34. (Try the edit without a restart; if behaviour seems wrong at the next step, gdk restart rails-web.)

  2. Reload the page. No visible change (everything was already at the new version), but the FF is now ignored: changing it has no effect, because CACHE_COMMONMARK_VERSION_PREVIOUS_SHIFTED is nil.

  3. Delete the FF to keep your database tidy:

    [15] pry(main)> Feature.remove(:markdown_cache_stochastic_rollout_34)
=> true

Bump #2: roll forward by re-adding ColorFilter

Now repeat the loop with the change made in the opposite direction. This confirms a second cache version bump works on top of a freshly-completed first one.

  1. Uncomment Filter::ColorFilter in lib/banzai/pipeline/gfm_pipeline.rb.

  2. In lib/gitlab/markdown_cache.rb:

    • Bump CACHE_COMMONMARK_VERSION from 34 to 35.
    • Set CACHE_COMMONMARK_VERSION_PREVIOUS = 34.

  3. Reload. No chips (FF unset, all reads roll "previous").

  4. Feature.enable_percentage_of_time(:markdown_cache_stochastic_rollout_35, 10).

  5. Reload repeatedly. Chips reappear on more notes with each refresh. (It may take a bit vis-à-vis code reloading.)

  6. Feature.enable_percentage_of_time(:markdown_cache_stochastic_rollout_35, 100).

  7. Reload. Every note has its chip back by now.

    [34] pry(main)> Issue.first.notes.pluck(:cached_markdown_version).tally
=> {2293760=>49}

  8. Complete: set CACHE_COMMONMARK_VERSION_PREVIOUS = nil in markdown_cache.rb and execute Feature.remove(:markdown_cache_stochastic_rollout_35) at the console.

  9. Reload. No change. Steady again and ready for the next bump.

Teardown

Revert all local edits. Some objects are going to have weird Markdown cache versions, but it doesn't matter super much: they'll get updated back to the actual current version on next edit. If you want to re-render things, you can try such fun actions as:

[7] pry(main)> Note.pluck(:cached_markdown_version).tally
=> {2162688=>1682, 2293760=>48}
[8] pry(main)> Note.update!(cached_markdown_version: 0).count

[About a minute or two passes.]

=> 1730
[9] pry(main)> Note.pluck(:cached_markdown_version).tally
=> {2162688=>1730}

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Asherah Connor

Merge request reports