Skip to content

Conversation

pm5
Copy link
Contributor

@pm5 pm5 commented Mar 31, 2025

Summary

It PR adds a query_settings field in the configuration of clickhouse sink, with a few configurations for asynchronous inserts.

Change Type

  • Bug fix
  • New feature
  • Non-functional (chore, refactoring, docs)
  • Performance

Is this a breaking change?

  • Yes
  • No

How did you test this PR?

I use the following Vector config to test it:

sources:
  demo_logs:
    type: demo_logs
    format: syslog
    count: 10
    interval: 0.0

sinks:
  clickhouse:
    inputs: ["demo_logs"]
    type: clickhouse
    endpoint: http://localhost:8123/
    database: default
    table: demo_logs
    skip_unknown_fields: true
    auth:
      user: default
      password: ''
      strategy: basic
    query_settings:
      async_insert: true
      wait_for_async_insert: true
      async_insert_max_data_size: 1000000
      async_insert_max_query_number: 1000000

Start ClickHouse, enable asynchronous logs, create a demo_logs table:

CREATE TABLE default.demo_logs
(
    `host` String,
    `message` String,
    `service` String,
    `source_type` String,
    `timestamp` String
)
ENGINE = MergeTree
ORDER BY timestamp

Run Vector. And then check ClickHouse logs for asynchronous inserts:

select now(), * from system.asynchronous_insert_log order by event_time format Vertical

You should see the queries done by Vector.

Does this PR include user facing changes?

  • Yes. Please add a changelog fragment based on our guidelines.
  • No. A maintainer will apply the "no-changelog" label to this PR.

Checklist

  • Please read our Vector contributor resources.
    • make check-all is a good command to run locally. This check is
      defined here. Some of these
      checks might not be relevant to your PR. For Rust changes, at the very least you should run:
      • cargo fmt --all
      • cargo clippy --workspace --all-targets -- -D warnings
      • cargo nextest run --workspace (alternatively, you can run cargo test --all)
  • If this PR introduces changes Vector dependencies (modifies Cargo.lock), please
    run dd-rust-license-tool write to regenerate the license inventory and commit the changes (if any). More details here.

References

@pm5 pm5 requested a review from a team as a code owner March 31, 2025 14:27
@bits-bot
Copy link

bits-bot commented Mar 31, 2025

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added the domain: sinks Anything related to the Vector's sinks label Mar 31, 2025
@pm5 pm5 changed the title feat(sink: clickhouse) #22373: add query_settings to clickhouse sink feat(sink: clickhouse): add query_settings to clickhouse sink Mar 31, 2025
@pm5 pm5 force-pushed the feat-clickhouse-querysettings branch 2 times, most recently from a017958 to a7b55af Compare March 31, 2025 14:41
@pront pront added the sink: clickhouse Anything `clickhouse` sink related label Mar 31, 2025
@pm5
Copy link
Contributor Author

pm5 commented Apr 7, 2025

Hi @pront ! Thanks for updating the info about this PR. Is there anything I can add to help with the reviewing?

@pront pront changed the title feat(sink: clickhouse): add query_settings to clickhouse sink feat(clickhouse sink): add query_settings to clickhouse sink Apr 7, 2025
Copy link
Member

@pront pront left a comment

Choose a reason for hiding this comment

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

Thank you @pm5, this looks good overall. Left a small comment.

@pm5 pm5 force-pushed the feat-clickhouse-querysettings branch from 232c2c0 to 17e0442 Compare April 9, 2025 04:29
@pm5
Copy link
Contributor Author

pm5 commented Apr 10, 2025

Thanks @pront . Let me know what you think of the current version.

@pm5 pm5 force-pushed the feat-clickhouse-querysettings branch 2 times, most recently from 411a447 to 4440d53 Compare April 14, 2025 16:34
@pm5 pm5 force-pushed the feat-clickhouse-querysettings branch 2 times, most recently from d48b1dc to 0f93164 Compare April 14, 2025 17:57
@pm5 pm5 force-pushed the feat-clickhouse-querysettings branch from 0f93164 to 9906d10 Compare April 14, 2025 17:59
@pm5
Copy link
Contributor Author

pm5 commented Apr 15, 2025

I found a problem with bool type options and fixed it. Also asked clippy to allow more arguments on set_uri_query.

@pront
Copy link
Member

pront commented Apr 15, 2025

I found a problem with bool type options and fixed it. Also asked clippy to allow more arguments on set_uri_query.

Please do not force push because I have to review the whole PR every time 😅

@pront pront force-pushed the master branch 4 times, most recently from 1720078 to ffe54be Compare July 10, 2025 15:43
@pront pront enabled auto-merge July 11, 2025 14:30
@pront pront disabled auto-merge July 11, 2025 14:38
@thomasqueirozb thomasqueirozb requested a review from a team as a code owner July 11, 2025 14:41
@github-actions github-actions bot added the domain: external docs Anything related to Vector's external, public documentation label Jul 11, 2025
@pront pront enabled auto-merge July 11, 2025 14:45
@pront pront added this pull request to the merge queue Jul 11, 2025
Merged via the queue into vectordotdev:master with commit f83b7e0 Jul 11, 2025
42 checks passed
@pront
Copy link
Member

pront commented Jul 11, 2025

I went ahead and merged this @pm5 because I think it's a cool new feature. We have plenty of time until next release if there's something you would like to tweak. Current state LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain: external docs Anything related to Vector's external, public documentation domain: sinks Anything related to the Vector's sinks sink: clickhouse Anything `clickhouse` sink related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature request: ClickHouse query settings

5 participants