-
Notifications
You must be signed in to change notification settings - Fork 6.3k
Improve metadata storage of chart and host labels #21492
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
base: master
Are you sure you want to change the base?
Conversation
|
@cubic-dev-ai review PR |
@stelfrag I have started the AI code review. It will take a few minutes to complete. |
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.
No issues found across 3 files
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.
Pull request overview
This PR refactors the metadata label storage mechanism for both host and chart labels by replacing SQL string concatenation with prepared statements and implementing a two-phase approach to avoid database I/O while holding spinlocks.
Changes:
- Introduced
store_labels()function with prepared statements to handle both host and chart labels via a unified code path - Replaced SQL string building with UPSERT statements (INSERT ... ON CONFLICT) for atomic label updates
- Removed
work_bufferparameter from multiple functions, simplifying the API
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/database/sqlite/sqlite_metadata.h | Updated store_host_info_and_metadata() signature to remove work_buffer parameter |
| src/database/sqlite/sqlite_metadata.c | Added new store_labels() function with two-phase label collection; refactored label storage to use prepared statements; removed work buffer usage throughout |
| src/database/rrdhost.c | Updated call to store_host_info_and_metadata() to remove buffer creation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…atements * Replace SQL string concatenation with prepared statements for label storage. * Simplify host and chart label update logic by removing work buffer usage. * Introduce `store_labels` function for unified label handling. * Improve code readability and maintainability by reducing redundant callback implementations.
- Introduced a reusable buffer for building SQL statements. - Replaced per-label operations with multi-row inserts using prepared statements. - Enhanced scalability by adding a batch size definition.
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.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Summary by cubic
Refactors label metadata storage to use prepared statements with batched inserts and a single code path for host and chart labels. Collects labels under spinlock and writes them after release to avoid DB I/O while locked.
Written for commit c2fac18. Summary will update on new commits.