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

[v10] Prevent races creating web api session context #23736

Merged
merged 1 commit into from Mar 29, 2023

Conversation

rosstimothy
Copy link
Contributor

Backport #23691 to branch/v10

Closes #23533 and #20963.

There was a race to create the `web.SessionContext` for a session
when multiple Proxies are behind a load balancer. Only the Proxy
that processes the login will have a `web.SessionContext` created
for the session. Any subsequent requests to the other Proxies in
the pool would create one if the request was authenticated. However,
multiple requests within a short succession could cause a
single Proxy to create multiple `web.SessionContext` for a single
session. When that happens the most recently created `web.SessionContext`
gets saved and the previous `web.SessonContext` gets closed. Closing
causes the `auth.Client` to be closed, which causes any active requests
for that client to return with a `grpc: client connection is closing`
error. This manifests in a single request from the web UI to fail
and depending on the request, for a banner to be displayed with the
error. Refreshing the page or navigating to another page would
resolve the problem because the most recent `web.SessionContext`
would be used with the still open `auth.Client`.

To prevent `web.Handler.AuthenticateRequest` from racing to create
the `web.SessionContext` a `singleflight.Group` was added to the
`web.sessionCache`. When multiple requests come in for the same
session they now will all use the first `web.SessionContext` to
be created instead of each creating their own.
@rosstimothy rosstimothy marked this pull request as ready for review March 28, 2023 21:47
@rosstimothy rosstimothy added this pull request to the merge queue Mar 28, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 29, 2023
@rosstimothy rosstimothy added this pull request to the merge queue Mar 29, 2023
Merged via the queue into branch/v10 with commit ed4e587 Mar 29, 2023
18 checks passed
@rosstimothy rosstimothy deleted the tross/backport-23691/v10 branch March 29, 2023 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants