Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Controlled upgrades #47877

Merged
merged 12 commits into from
Sep 13, 2024
Merged

Controlled upgrades #47877

merged 12 commits into from
Sep 13, 2024

Conversation

dpsutton
Copy link
Contributor

@dpsutton dpsutton commented Sep 11, 2024

Let's add a rollout number 0-100 on latest

Version information has two keys:

{:latest {:version "v1.50.25",
          :released "2024-09-10",
          :patch true,
          :highlights [,,,]},
 :older [{:version "v1.50.24",
          :released "2024-09-03",
          :patch true,
          :highlights [,,,]}]}

The proposal here is that we add a rollout to the latest map, and then we only show the latest to the frontend (and therefore the upgrade) if some number of the local instance is lower than the rollout number.

{:latest {:version "v1.50.25",
          :released "2024-09-10",
          :rollout 50 ;; number between 0 - 100 <--------- new
          :patch true,
          :highlights [,,,]},
 :older [{:version "v1.50.24",
          :released "2024-09-03",
          :patch true,
          :highlights [,,,]}]}
image

This derives the rollout threshold of the instance from the site-uuid which is constant for all instances in a cluster.

(defsetting upgrade-threshold
  (deferred-tru "Threshold (value in 0-100) indicating at which treshold it should offer an upgrade to the latest major version.")
  :visibility :internal
  :export?    false
  :type       :integer
  :setter     :none
  :getter     (fn []
                (-> (site-uuid) hash (mod 100))))

(and of course overridable with MB_UPGRADE_THRESHOLD=90 java -jar ... if they want to be in a higher cohort)

This lets us uniformly distribute self-hosted instances into buckets of 0-100. And as we increase the rollout number of the version-info json file, more instances will show the upgrade information in the application.

❯ http get "http://localhost:3000/api/setting/version-info" x-api-key:$API_KEY | jq 'keys'
[
  "older"
]

## vs when it should show
❯ http get "http://localhost:3000/api/setting/version-info" x-api-key:$API_KEY | jq 'keys'
[
  "latest",
  "older"
]

This really only matters across major upgrades. We always show upgrades of minor upgrades in the same major.
Ex: 0.49.5 will always show a banner for a latest of 0.49.6
Of course 0.49.5 will compare rollout thresholds on whether to show an update to the latest of 0.50.2
Also, 0.49.5 will compare rollout thresholds on whether to show an update to the latest of 0.51.2 (2 majors). We can make this show the latest 50 in the future if we want. (and the same scenario for more exaggerated version jumps, 49 -> 83 ,etc).

See this explanation in notion of how it worked running it 200 times.

Started a jar up 200 times, fetching versions that included a 25% rollout threshold and had the following results:
image

site-uuid, threshold, upgrade-available, latest-version
35095708-ad3f-42b2-8e26-65b475ec5cd7, 2, true, 1.51.1
a247324a-82c1-41e1-9f91-7016a66e9014, 87, false, 
8072a554-3ded-4a4f-83ec-0d363e7f0deb, 23, true, 1.51.1
b40db850-028e-4d17-8806-0d27758395f6, 67, false, 
a466def7-fa50-4cf7-a2ee-8b31b422749a, 20, true, 1.51.1
5effa39b-9065-49ca-b686-f17eaecf4198, 85, false, 
4158efe4-3703-4372-b94a-9458f975fa7c, 55, false, 

@iethree iethree added the backport Automatically create PR on current release branch on merge label Sep 12, 2024
@dpsutton dpsutton marked this pull request as ready for review September 12, 2024 17:48
@dpsutton dpsutton requested review from a team September 12, 2024 17:52
src/metabase/public_settings.clj Outdated Show resolved Hide resolved
return (
<div>
<OnLatestVersionMessage>
{t`You're running Metabase ${currentVersion}`}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Context string suggestion: c("{0} is a version number."}

@iethree iethree self-assigned this Sep 12, 2024
dpsutton and others added 9 commits September 12, 2024 14:04
- new setting upgrade-threshold (`MB_UPGRADE_THRESHOLD`)
  number 0-100
- conditionally removing latest from upgrade checks

AS OF RIGHT NOW IT ALWAYS REMOVES the latest.
Should help FE make this optional
nothing is sacred in the settings order. no reason. i think i was just
trying to minimize the diff, which a dumb and ignoble goal.
:type :integer
:setter :none
:getter (fn []
(-> (site-uuid) hash (mod 100))))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mentioned that in Slack, but maybe we don't want this number to be THAT stable and adding a version info (or something like this) to re-hash our distribution after every upgrade is the better way?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated! good idea

hopefully helps rotate people from early to late, and late to early
across major versions. Idea from sanya and quite nice!
@dpsutton dpsutton merged commit 1addcc1 into master Sep 13, 2024
121 checks passed
@dpsutton dpsutton deleted the controlled-upgrades branch September 13, 2024 14:58
iethree added a commit that referenced this pull request Sep 13, 2024
* Initial commit of controlled upgrades

- new setting upgrade-threshold (`MB_UPGRADE_THRESHOLD`)
  number 0-100
- conditionally removing latest from upgrade checks

AS OF RIGHT NOW IT ALWAYS REMOVES the latest.
Should help FE make this optional

* dumb mistake

* handle no latest version info

* implement and basic test

* More tests, refactor name

* more tests

* cljfmt does not like commas in source code.

* Just move site-uuid to the declaration and don't declare it

nothing is sacred in the settings order. no reason. i think i was just
trying to minimize the diff, which a dumb and ignoble goal.

* add context strings

* update unit tests

* use display version

* Have upgrade threshold include current major version

hopefully helps rotate people from early to late, and late to early
across major versions. Idea from sanya and quite nice!

---------

Co-authored-by: Ryan Laurie <iethree@gmail.com>
# Conflicts:
#	frontend/src/metabase/admin/settings/components/SettingsUpdatesForm/VersionUpdateNotice/VersionUpdateNotice.tsx
@github-actions github-actions bot added this to the 0.50.26 milestone Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Automatically create PR on current release branch on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants