docs: query cache with more settings avoid query crashes #403
Conversation
Reviewer's Guide by SourceryThis PR enhances query caching configuration by adding additional settings to prevent query crashes. The changes include adding new query cache parameters in the ClickHouse configuration and updating the query settings in React components to handle system tables and nondeterministic functions properly. Class diagram for updated query settingsclassDiagram
class ClickHouseSettings {
+int use_query_cache
+int query_cache_ttl
+int query_cache_max_entries
+String query_cache_system_table_handling
+String query_cache_nondeterministic_function_handling
}
class ChartQueryDuration {
+ClickHouseSettings clickhouse_settings
}
class CountBadge {
+ClickHouseSettings clickhouse_settings
}
ClickHouseSettings <|-- ChartQueryDuration
ClickHouseSettings <|-- CountBadge
note for ClickHouseSettings "New settings added for system table and nondeterministic function handling"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Hey @duyet - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Review instructions: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟡 Documentation: 1 issue found
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -46,6 +46,8 @@ Suggested role for "monitoring" user must have these privileges on `system` data | |||
| <use_query_cache>1</use_query_cache> | |||
| <query_cache_ttl>50</query_cache_ttl> | |||
| <query_cache_max_entries>0</query_cache_max_entries> | |||
There was a problem hiding this comment.
question (bug_risk): Could you clarify if setting query_cache_max_entries to 0 is intentional?
A value of 0 might indicate unlimited entries, but it would be helpful to confirm this is the desired behavior and won't cause any memory management issues.
| <query_cache_system_table_handling>save</query_cache_system_table_handling> | ||
| <query_cache_nondeterministic_function_handling>save</query_cache_nondeterministic_function_handling> |
There was a problem hiding this comment.
suggestion (documentation): Consider adding brief descriptions for the new cache handling parameters
It would be helpful to document the available options for these parameters and their implications for query caching behavior.
<!-- Options: save|skip - Controls whether queries with system tables are cached -->
<query_cache_system_table_handling>save</query_cache_system_table_handling>
<!-- Options: save|skip - Controls caching of queries with non-deterministic functions -->
<query_cache_nondeterministic_function_handling>save</query_cache_nondeterministic_function_handling>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #403 +/- ##
=======================================
Coverage ? 78.34%
=======================================
Files ? 5
Lines ? 157
Branches ? 58
=======================================
Hits ? 123
Misses ? 31
Partials ? 3 ☔ View full report in Codecov by Sentry. |
Bundle ReportChanges will increase total bundle size by 95 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
|
Related #400
Related #401
Summary by Sourcery
Enhance query cache configuration by adding settings for system table and nondeterministic function handling to prevent query crashes and update relevant documentation.
Enhancements:
Documentation: