-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
query: fix deadlock in endpointset #4795
Conversation
Avoid RLock()ing twice as described here: thanos-io#4766 (comment) (due to https://stackoverflow.com/questions/30547916/goroutine-blocks-when-calling-rwmutex-rlock-twice-after-an-rwmutex-unlock/30549188). Fix it by removing HasClients() and simply changing it with `er.clients != nil`. Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick fix!
Avoid RLock()ing twice as described here: thanos-io#4766 (comment) (due to https://stackoverflow.com/questions/30547916/goroutine-blocks-when-calling-rwmutex-rlock-twice-after-an-rwmutex-unlock/30549188). Fix it by removing HasClients() and simply changing it with `er.clients != nil`. Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Hi @GiedriusS @squat , I found at least 4 people report this bug, it critical because query will halt . Could we released a miro version like 0.23.2, just cherry pick this import pr. Thanks very much. |
This PR is in the release candidate for 0.24.0. i think that's enough. Or do you think it's so severe that we need to back port the fix? |
@squat I think it's severe to do that. Someone use 0.23.x always face this deadlock issue, the query api will halt and process will not auto reboot, It will be nice to have 0.23.2 for 0.23.x series. 😄 I am also face this issue on production upgrade, other case like https://cloud-native.slack.com/archives/CK5RSSC10/p1637585247351700 https://cloud-native.slack.com/archives/CK5RSSC10/p1637265253335900 . |
Avoid RLock()ing twice as described here: thanos-io#4766 (comment) (due to https://stackoverflow.com/questions/30547916/goroutine-blocks-when-calling-rwmutex-rlock-twice-after-an-rwmutex-unlock/30549188). Fix it by removing HasClients() and simply changing it with `er.clients != nil`. Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Avoid RLock()ing twice as described here: thanos-io#4766 (comment) (due to https://stackoverflow.com/questions/30547916/goroutine-blocks-when-calling-rwmutex-rlock-twice-after-an-rwmutex-unlock/30549188). Fix it by removing HasClients() and simply changing it with `er.clients != nil`. Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com> Signed-off-by: Aymeric <aymeric.daurelle@cdiscount.com>
Avoid RLock()ing twice as described here: #4766 (comment) (due to https://stackoverflow.com/questions/30547916/goroutine-blocks-when-calling-rwmutex-rlock-twice-after-an-rwmutex-unlock/30549188). Fix it by removing HasClients() and simply changing it with `er.clients != nil`. Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com> Signed-off-by: Aymeric <aymeric.daurelle@cdiscount.com> Co-authored-by: Giedrius Statkevičius <giedriuswork@gmail.com>
Mainly to address thanos-io/thanos#4795 - which is happening frequently in prod-aws/sys-mon
Mainly to address thanos-io/thanos#4795 - which is happening frequently in prod-aws/sys-mon
Issue: https://issues.redhat.com/browse/OCPBUGS-2037 Problem: thanos-io/thanos#4766 Solution: update to 0.23.2 that contains the patch thanos-io/thanos#4795 Signed-off-by: JoaoBraveCoding <jmarcal@redhat.com>
Avoid RLock()ing twice as described here:
#4766 (comment)
(due to
https://stackoverflow.com/questions/30547916/goroutine-blocks-when-calling-rwmutex-rlock-twice-after-an-rwmutex-unlock/30549188).
Fix it by removing HasClients() and simply changing it with
er.clients != nil
.Closes #4766.
Signed-off-by: Giedrius Statkevičius giedrius.statkevicius@vinted.com