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

fix(api): make stick sessions actually work and make them non-racy #12665

Merged
merged 3 commits into from
Nov 1, 2024

Conversation

Stebalien
Copy link
Member

@Stebalien Stebalien commented Nov 1, 2024

Related Issues

We apparently have a way to specify that all "related" requests should go to the same node. However:

  1. It didn't work at all. All future requests would go to the first successful node from the first request. Because that's how stack variables work.
  2. It was racy if the context was re-used concurrently. But only the first time, see point 1.

Proposed Changes

Use an atomic, get rid of a few of the odd decisions, and write a test.

Additional Info

Checklist

Before you mark the PR ready for review, please make sure that:

We apparently have a way to specify that all "related" requests should
go to the same node. However:

1. It didn't work at all. All future requests would go to the first successful
node from the first request. Because that's how stack variables work.
2. It was racy if the context was re-used concurrently. But only the
first time, see point 1.
@Stebalien Stebalien force-pushed the steb/fix-sticky-race branch from c284b42 to 7640282 Compare November 1, 2024 02:26
1. Test whether or not it works.
2. Test stickiness.
@Stebalien Stebalien force-pushed the steb/fix-sticky-race branch from 7640282 to 1e721d8 Compare November 1, 2024 02:44
cli/util/api.go Outdated Show resolved Hide resolved
Copy link
Contributor

@magik6k magik6k left a comment

Choose a reason for hiding this comment

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

Reads a little more correctly now

@Stebalien Stebalien enabled auto-merge (squash) November 1, 2024 04:04
@Stebalien Stebalien merged commit 2657108 into master Nov 1, 2024
83 checks passed
@Stebalien Stebalien deleted the steb/fix-sticky-race branch November 1, 2024 04:07
rjan90 pushed a commit that referenced this pull request Nov 4, 2024
…12665)

* fix(api): make stick sessions actually work and make them non-racy

We apparently have a way to specify that all "related" requests should
go to the same node. However:

1. It didn't work at all. All future requests would go to the first successful
node from the first request. Because that's how stack variables work.
2. It was racy if the context was re-used concurrently. But only the
first time, see point 1.

* test(api): test the API merge proxy

1. Test whether or not it works.
2. Test stickiness.

* fix(api): update OnSingleNode documentation
rjan90 pushed a commit that referenced this pull request Nov 6, 2024
…12665)

* fix(api): make stick sessions actually work and make them non-racy

We apparently have a way to specify that all "related" requests should
go to the same node. However:

1. It didn't work at all. All future requests would go to the first successful
node from the first request. Because that's how stack variables work.
2. It was racy if the context was re-used concurrently. But only the
first time, see point 1.

* test(api): test the API merge proxy

1. Test whether or not it works.
2. Test stickiness.

* fix(api): update OnSingleNode documentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ☑️ Done (Archive)
Development

Successfully merging this pull request may close these issues.

3 participants