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

Support more regional workspace cluster #7490

Closed
wants to merge 2 commits into from
Closed

Conversation

csweichel
Copy link
Contributor

Description

Today workspace cluster choice is guided solely by the location of the application cluster the workspace is started from. This PR introduces an admission preference (introduced in #6164) which adds more regional support.

The current implementation in this branch is a rather rough first start. It does two things:

  1. introduce the region admission preferences on workspace clusters. Each such admission preference carries a "round trip time endpoint" which is used to measure the RTT of a user to a particular region. We assume that if there are multiple clusters within the same "region" that their "RTT endpoints" yield equivalent results.
  2. make the start workspace page measure the RTT and update the cluster preference for the user. This part I'm least happy with, because it's
  • super spammy and makes many more requests than it should
  • comes too late: only after the first workspace is running will this be done

It's particularly with point 2 that I'd need some support @gitpod-io/engineering-meta :)

Related Issue(s)

Fixes #5596

How to test

  1. remove the default cluster registration by editing the WSMAN_CFG env var on server setting it to W10K (base64 for [])
  2. register a the ws-manager of the preview environment with gpctl clusters register ... make sure it's governed
  3. register a ws-manager of another preview environment
  4. use gpctl clusters update admission-preference add ... to point one cluster to a "slow" RTT endpoint, e.g. ws-us25.gitpod.io, and one to a "fast" RTT endpoint, e.g. ws-eu25.gitpod.io (depending on where in the world you're located)
  5. start a workspace and observe the correct cluster choice

Release Notes

Support more regional workspace cluster

Documentation

@roboquat roboquat added release-note do-not-merge/work-in-progress team: webapp Issue belongs to the WebApp team team: workspace Issue belongs to the Workspace team labels Jan 6, 2022
@roboquat
Copy link
Contributor

roboquat commented Jan 6, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please ask for approval from csweichel and additionally assign geropl after the PR has been reviewed.
You can assign the PR to them by writing /assign @geropl in a comment when ready.

Associated issue: #6164

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@csweichel csweichel marked this pull request as ready for review January 7, 2022 09:37
@csweichel csweichel changed the title Support "hyper-regional" workspace cluster Support more regional workspace cluster Jan 7, 2022
Copy link
Contributor

@jankeromnes jankeromnes left a comment

Choose a reason for hiding this comment

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

Exciting to see progress on this! ⚡ 👏 Many thanks for moving this forward. 🙏

Added a few drive-by comments in the dashboard part. Please disregard if unhelpful.

components/dashboard/src/start/StartPage.tsx Outdated Show resolved Hide resolved
components/dashboard/src/start/choose-region.ts Outdated Show resolved Hide resolved
components/dashboard/src/Login.tsx Outdated Show resolved Hide resolved
components/gitpod-protocol/src/protocol.ts Outdated Show resolved Hide resolved
@JanKoehnlein
Copy link
Contributor

JanKoehnlein commented Jan 7, 2022

My thoughts on this

  1. Why not store the lastMeasurement date on the client (e.g. localStorage) instead of in the DB? It'd save one server call and allow to move this up in the component hierarchy as @jankeromnes recommends
  2. Await on the measurement in case there is none yet. IMHO it's better to wait a few more seconds (AFAIKS its max 5s which could also be fine tuned) for the first workspace than being confronted with a horrible UX due to long latencies. We could show that to the user as a phase to raise acceptance if it takes longer (who would object a "Finding the best server for you")

@csweichel csweichel force-pushed the cw/regional-ws-cluster branch 5 times, most recently from f60553a to 5b1de04 Compare January 10, 2022 12:30
[server] Test RTT for available workspace cluster only

[dashboard] Improve RTT measurement

[dashboard] enable RTT measurement
@codecov
Copy link

codecov bot commented Jan 19, 2022

Codecov Report

Merging #7490 (98ee125) into main (92a9fd5) will increase coverage by 1.69%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            main    #7490      +/-   ##
=========================================
+ Coverage   8.68%   10.38%   +1.69%     
=========================================
  Files         33       18      -15     
  Lines       2325      992    -1333     
=========================================
- Hits         202      103      -99     
+ Misses      2119      888    -1231     
+ Partials       4        1       -3     
Flag Coverage Δ
components-gitpod-cli-app 10.38% <ø> (ø)
components-local-app-app-darwin-amd64 ?
components-local-app-app-darwin-arm64 ?
components-local-app-app-linux-amd64 ?
components-local-app-app-linux-arm64 ?
components-local-app-app-windows-386 ?
components-local-app-app-windows-amd64 ?
components-local-app-app-windows-arm64 ?
installer-raw-app ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
installer/pkg/common/objects.go
installer/pkg/common/common.go
...components/ws-manager/unpriviledged-rolebinding.go
installer/pkg/common/display.go
...staller/pkg/components/ws-manager/networkpolicy.go
installer/pkg/common/render.go
installer/pkg/common/storage.go
installer/pkg/components/ws-manager/tlssecret.go
components/local-app/pkg/auth/auth.go
installer/pkg/common/ca.go
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 92a9fd5...98ee125. Read the comment docs.

@JanKoehnlein
Copy link
Contributor

Code LGTM now, also squashed it.

I admit that I am stuck in the testing steps as I don't know how to register a different preview env's ws-manager: What is the URL? The one of the local preview env is a dns:// URL which I assume won't work.

@stale
Copy link

stale bot commented Jan 30, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the meta: stale This issue/PR is stale and will be closed soon label Jan 30, 2022
@kylos101
Copy link
Contributor

kylos101 commented Feb 1, 2022

Closing per conversation with @csweichel

@kylos101 kylos101 closed this Feb 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/work-in-progress meta: stale This issue/PR is stale and will be closed soon release-note size/XXL team: webapp Issue belongs to the WebApp team team: workspace Issue belongs to the Workspace team
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Automatically choose workspace-cluster based on lowest latency.
5 participants