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

server: fix semaphore release #6737

Merged
merged 1 commit into from
Oct 18, 2023
Merged

server: fix semaphore release #6737

merged 1 commit into from
Oct 18, 2023

Conversation

bboreham
Copy link
Contributor

@bboreham bboreham commented Oct 18, 2023

This is to fix a bug in v1.57.1 where servers will run very sluggishly, and also take ages to shut down.

The acquire and release functions should have opposite impacts on the count.

Credit to @colega who spotted the diff.

This was introduced in #6706, a backport of #6703, where the line is correct.
https://github.com/grpc/grpc-go/pull/6703/files#diff-366e46a40f6f60b4f7614eb0976bb51820364bf5ca6ccc4787eb49d7bdbef3e6R2098

Also correct in the 1.56 backport: https://github.com/grpc/grpc-go/pull/6708/files#diff-366e46a40f6f60b4f7614eb0976bb51820364bf5ca6ccc4787eb49d7bdbef3e6R2073

RELEASE NOTES: n/a

The acquire and release functions should have opposite impacts on the
count.

Credit to @colega who spotted the diff.

Signed-off-by: Bryan Boreham <[email protected]>
colega added a commit to grafana/mimir that referenced this pull request Oct 18, 2023
This includes fix from grpc/grpc-go#6737

Should fix #6385

Signed-off-by: Oleg Zaytsev <[email protected]>
colega added a commit to grafana/mimir that referenced this pull request Oct 18, 2023
* Update grpc to 1.57.2-dev

This includes fix from grpc/grpc-go#6737

Should fix #6385

Signed-off-by: Oleg Zaytsev <[email protected]>

* Update CHANGELOG.md

Signed-off-by: Oleg Zaytsev <[email protected]>

---------

Signed-off-by: Oleg Zaytsev <[email protected]>
Copy link
Member

@dfawley dfawley left a comment

Choose a reason for hiding this comment

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

👀 Not sure how this happened, sorry!

@dfawley dfawley added this to the 1.57 Release milestone Oct 18, 2023
@bboreham
Copy link
Contributor Author

I looked at the CI failure; can’t see how it can be related to this PR.

@dfawley
Copy link
Member

dfawley commented Oct 18, 2023

I looked at the CI failure; can’t see how it can be related to this PR.

It isn't related, and it's an optional checker so won't block merging. However, we do require 2 maintainers to review PRs before merging, which is why I haven't merged it yet.

@dfawley dfawley merged commit 4652392 into grpc:v1.57.x Oct 18, 2023
10 of 11 checks passed
smira added a commit to smira/talos that referenced this pull request Oct 19, 2023
See grpc/grpc-go#6737

Note: this isn't a problem in `main`, as we're using 1.58 there, and it
never had this bug.

Signed-off-by: Andrey Smirnov <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants