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

Add deadlock prevention during metric registration #1079

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

suligap
Copy link
Contributor

@suligap suligap commented Dec 3, 2024

Detect deadlocks during the library misuse, eg. by injecting code into the critical sections that itself might want to obtain the relevant lock.

A follow up to #1076.

@csmarchbanks
Copy link
Member

Just want to say I have been busy this week and am out next week, but I will get to this, and thank you for your contributions!

Detects and prevents deadlocks during the library misuse, eg. by
injecting code into the critical sections that itself might want to
obtain the relevant lock.

A follow up to prometheus#1076.

Signed-off-by: Przemysław Suliga <[email protected]>
@suligap suligap force-pushed the deadlock-prevention branch from 91d9893 to 4235ae2 Compare December 8, 2024 15:38
It's hard to justify the overhead of double locking there.

Signed-off-by: Przemysław Suliga <[email protected]>
@suligap suligap force-pushed the deadlock-prevention branch from 4235ae2 to c0d5593 Compare December 8, 2024 15:38
@suligap suligap changed the title Add deadlock detection Add deadlock prevention during metric registration Dec 8, 2024
@suligap
Copy link
Contributor Author

suligap commented Dec 8, 2024

No worries, nothing urgent here and thanks. I thought about it a bit more and ended up pulling out the deadlock prevention from the "hottest" path: metrics.py and values.py. It's quite hard to justify the overhead of this double locking approach there. But I think it's not an issue in registry.py.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants