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
Context lock contention fix #55121
Context lock contention fix #55121
Conversation
This is an automated comment for commit 71cb645 with description of existing statuses. It's updated for the latest CI running ❌ Click here to open a full report in a separate page Successful checks
|
Benchmark to simulate complex short-running query, this is not worst case scenario.
Before:
After:
|
src/Interpreters/Context.cpp
Outdated
@@ -172,15 +172,42 @@ namespace ErrorCodes | |||
} while (false) \ | |||
|
|||
|
|||
/// Thread safe object initializer using double-checked locking | |||
class Initializer |
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.
Looks ok. Maybe we can think about replacing it with std::call_once
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.
Why not something like
void initialize()
{
static Initializer ...
}
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.
I did not check all the lock calls, but overall, it looks good. Let\s merge.
…532d1b73e1236ed54ce9e182598ee Cherry pick #55121 to 23.9: Context lock contention fix
porting ClickHouse/ClickHouse#55121 fix dup lock in calculateAccessRightsWithLock
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fix contention on Context lock, this significantly improves performance for a lot of short-running concurrent queries.