Skip to content

Insert profile event fix for async inserts with inline data#98962

Merged
npakeer merged 9 commits intoClickHouse:masterfrom
npakeer:insert_profile_event_fix
Mar 11, 2026
Merged

Insert profile event fix for async inserts with inline data#98962
npakeer merged 9 commits intoClickHouse:masterfrom
npakeer:insert_profile_event_fix

Conversation

@npakeer
Copy link
Copy Markdown
Contributor

@npakeer npakeer commented Mar 7, 2026

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

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

Increment InsertQuery ProfileEvent for async inserts. Fixes the issue - #98626

Documentation entry for user-facing changes

Async inserts with inline data are not being counted in InsertQuery profile event. This fix also increments InsertQuery profile event for async inserts with inline data .

  • Documentation is written (mandatory for new features)

Note

Low Risk
Small, metrics-only change in the async INSERT path plus a new integration test; low risk aside from potential monitoring/alerting deltas due to corrected counters.

Overview
Fixes a ProfileEvents counting gap for async INSERTs. When an async INSERT with inlined data is successfully enqueued (pushQueryWithInlinedData returns OK), executeQuery.cpp now also increments ProfileEvents::InsertQuery (in addition to existing async-specific counters).

Adds coverage. New integration test test_insert_query_profile_events asserts InsertQuery increases for both sync and async inserts while AsyncInsertQuery increases only for async inserts, with a minimal config (page_cache_size=0) for deterministic runs.

Written by Cursor Bugbot for commit 629aa28. This will update automatically on new commits. Configure here.

@npakeer npakeer marked this pull request as draft March 7, 2026 02:57
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Mar 7, 2026

Workflow [PR], commit [629aa28]

Summary:

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

@npakeer npakeer changed the title Insert profile event fix Insert profile event fix for async inserts with inline data Mar 7, 2026
@clickhouse-gh clickhouse-gh bot added the pr-bugfix Pull request with bugfix, not backported by default label Mar 7, 2026
@npakeer npakeer marked this pull request as ready for review March 9, 2026 15:35
@serxa serxa self-assigned this Mar 9, 2026
Co-authored-by: Sergei Trifonov <svtrifonov@gmail.com>
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Mar 10, 2026

LLVM Coverage Report

Metric Baseline Current Δ
Lines 83.20% 83.80% +0.60%
Functions 23.60% 23.90% +0.30%
Branches 75.30% 76.30% +1.00%

PR changed lines: PR changed-lines coverage: 100.00% (7/7)
Diff coverage report
Uncovered code

if (result.status == AsynchronousInsertQueue::PushResult::OK)
{
// Increment InsertQuery for async insert with inline data
ProfileEvents::increment(ProfileEvents::InsertQuery);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why don't we do it at the same place as for ProfileEvents::AsyncInsertQuery?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yakov-olkhovskiy The problem is happening only for async inserts with inline data. If we increment InsertQuery at the same place as AsyncInsertQuery, then InsertQuery will be double incremented for non inline data. For non inline data, it seems to go through InterpreterFactory.get() where the profile event event is incremented.

If my understanding is correct.

Inline async inserts go through here

Non In line async inserts goes through this. See last line of selection here:

InterpreterFactory.get() is incrementing profile event for noninline data. I did test for non inline async data, and InsertQuery is incrementing correctly. Let me know if you still have any questions.

Copy link
Copy Markdown
Contributor Author

@npakeer npakeer Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For more clarification, the data for non-inline data is being pushed directly to AsynchronousInsertQueue by TCPHandler here and here once response is given from executeQuery.cpp, and double counting will occur inside InterpreterFactory.get() and AsynchronousInsertQueue

@npakeer npakeer added this pull request to the merge queue Mar 11, 2026
Merged via the queue into ClickHouse:master with commit d80ab8d Mar 11, 2026
163 checks passed
@npakeer npakeer deleted the insert_profile_event_fix branch March 11, 2026 17:22
@robot-clickhouse-ci-1 robot-clickhouse-ci-1 added the pr-synced-to-cloud The PR is synced to the cloud repo label Mar 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-bugfix Pull request with bugfix, not backported by default 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.

4 participants