Skip to content

Reduce lock contention in QueryConditionCache#80247

Merged
rschu1ze merged 2 commits intoClickHouse:masterfrom
jiebinn:QueryConditionCache
May 18, 2025
Merged

Reduce lock contention in QueryConditionCache#80247
rschu1ze merged 2 commits intoClickHouse:masterfrom
jiebinn:QueryConditionCache

Conversation

@jiebinn
Copy link
Copy Markdown
Contributor

@jiebinn jiebinn commented May 15, 2025

Key Findings:
With the previous bottleneck high page faults resolved in jemalloc (#80245), we have identified a 76% hotspot in performance cycles native_queued_spin_lock_slowpath from QueryConditionCache::write in the latest build on the 2 x 240 vCPUs system. And we have discovered the absence of checks for mark_ranges and has_final_mark, causing all threads to attempt locking entry->mutex unnecessarily.

Solution:
Introduced a new method to QueryConditionCache to determine if cached query conditions need updating based on changes to underlying data or configuration.
This method avoids unnecessary cache rebuilds, ensuring data consistency and reducing lock contention, thereby improving concurrency in multi-threaded query environments.

Performance improvements:

  • Reduces the native_queued_spin_lock_slowpath hotspot of entry->mutex from 76% to 1% with Clickbench Q10 on a 2 x 240 vCPUs system
  • Increases QPS for Q10 and Q11 by 85% and 89% respectively
  • Improves overall performance across 43 queries with a geometric mean gain of 8.1%

Changelog category (leave one):

  • Performance Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Avoid unnecessary update and reduce lock contention in QueryConditionCache

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

…ached

query conditions need to be updated based on changes to underlying data or
configuration. This improves query performance by avoiding unnecessary cache
rebuilds while ensuring data consistency when conditions change. The method
also helps reduce unnecessary lock contention, improving concurrency in
multi-threaded query environments.

Performance improvements:
- Reduces the "native_queued_spin_lock_slowpath" hotspot of entry->mutex
  from 76% to 1% with Clickbench Q10 on a 2 x 240 vCPUs system
- Increases QPS for Q10 and Q11 by 85% and 89% respectively
- Improves overall performance across 43 queries with a geometric mean gain
  of 8.1%

Signed-off-by: Jiebin Sun <jiebin.sun@intel.com>
Reviewed-by: Wangyang Guo <wangyang.guo@intel.com>
@devcrafter devcrafter requested a review from rschu1ze May 15, 2025 11:02
@rschu1ze rschu1ze self-assigned this May 15, 2025
@rschu1ze
Copy link
Copy Markdown
Member

@jiebinn Could you please cherry-pick the topmost commit from here into this PR? I cleaned this PR a bit up but then couldn't push:

/data/ch2 (QueryConditionCache >) $ git push jiebinn jiebinn/QueryConditionCache
Total 0 (delta 0), reused 0 (delta 0), pack-reused 0
To https://github.com/jiebinn/ClickHouse.git
 ! [remote rejected]         jiebinn/QueryConditionCache -> jiebinn/QueryConditionCache (permission denied)
error: failed to push some refs to 'https://github.com/jiebinn/ClickHouse.git'

@jiebinn
Copy link
Copy Markdown
Contributor Author

jiebinn commented May 16, 2025

@rschu1ze , thanks. I have cherry-picked the first commit to this PR. The code now appears more organized and tidy.

@rschu1ze rschu1ze changed the title Avoid unnecessary update and reduce lock contention in QueryConditionCache Reduce lock contention in QueryConditionCache May 16, 2025
@rschu1ze rschu1ze added the can be tested Allows running workflows for external contributors label May 16, 2025
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented May 16, 2025

Workflow [PR], commit [5eec057]

@clickhouse-gh clickhouse-gh bot added the pr-performance Pull request with some performance improvements label May 16, 2025
@rschu1ze rschu1ze enabled auto-merge May 18, 2025 14:27
@rschu1ze rschu1ze added this pull request to the merge queue May 18, 2025
Merged via the queue into ClickHouse:master with commit 63058b4 May 18, 2025
117 of 121 checks passed
@robot-ch-test-poll2 robot-ch-test-poll2 added the pr-synced-to-cloud The PR is synced to the cloud repo label May 18, 2025
@rschu1ze rschu1ze added the pr-must-backport Pull request should be backported intentionally. Use this label with great care! label Jun 14, 2025
robot-ch-test-poll2 added a commit that referenced this pull request Jun 14, 2025
Cherry pick #80247 to 25.4: Reduce lock contention in QueryConditionCache
robot-ch-test-poll2 added a commit that referenced this pull request Jun 14, 2025
Cherry pick #80247 to 25.5: Reduce lock contention in QueryConditionCache
@robot-clickhouse-ci-2 robot-clickhouse-ci-2 added the pr-backports-created-cloud deprecated label, NOOP label Jun 14, 2025
robot-ch-test-poll4 added a commit that referenced this pull request Jun 14, 2025
Cherry pick #80247 to 25.3: Reduce lock contention in QueryConditionCache
rschu1ze added a commit that referenced this pull request Jun 14, 2025
Backport #80247 to 25.4: Reduce lock contention in QueryConditionCache
rschu1ze added a commit that referenced this pull request Jun 14, 2025
Backport #80247 to 25.5: Reduce lock contention in QueryConditionCache
@robot-ch-test-poll1 robot-ch-test-poll1 added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Jun 14, 2025
rschu1ze added a commit that referenced this pull request Jun 14, 2025
Backport #80247 to 25.3: Reduce lock contention in QueryConditionCache
@robot-clickhouse robot-clickhouse added the pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR label Jul 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

can be tested Allows running workflows for external contributors pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-backports-created-cloud deprecated label, NOOP pr-must-backport Pull request should be backported intentionally. Use this label with great care! pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR pr-performance Pull request with some performance improvements pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants