[DBMON-6881] Move clickhouse onto the DatabaseCheck job registry and cancellation lifecycle by eric-weaver · Pull Request #24934 · DataDog/integrations-core

eric-weaver · 2026-08-20T14:29:10Z

What does this PR do?

Moves clickhouse's seven DBMAsyncJobs onto the DatabaseCheck registry and cancellation lifecycle.

_register_async_jobs() builds and registers the jobs the config enables, following the postgres and mysql pattern: the attributes default to None in __init__, and the shared dbm gate is a single early return.
check() ends in self.run_async_jobs(self.tags) instead of a per-job fan-out, and no-ops once the check is cancelled.
The cancel() override is gone. Closing the main client and dropping the shared pool manager moved to shutdown(), which the base calls once, after the loops stop and never while check() is running.
Each job's cancel() override became shutdown(), which closes its client and drops _check. The check's shutdown() releases the query manager and health reporter; ClickhouseMetadata also drops the schema collector and ClickhouseQueryCompletions the explain plans helper, both of which hold the check.
Query samples, table metrics, and parts and merges now check _cancel_event before issuing each query, matching the query log job and postgres.

Motivation

The old cancel() signalled all seven jobs and then called .result() on all seven futures, blocking the Agent's unschedule thread inside cancel() — which AgentCheck.cancel() explicitly warns against. Inheriting the base protocol moves that wait after check() returns, so cancel() signals and gets out of the way.

One behavior change: a job's cancel() used to close its dedicated client immediately, which is what aborted an in-flight query. Teardown now waits for the loop to finish its current iteration, so a query already issued is bounded by the client read_timeout (10s) rather than cut short. The new cancel-event checks keep a cancelled job from starting further queries, so that wait is one query rather than one per query left in the tick.

Tests follow the postgres migration (#24933): the base suite owns the state machine, so per-job tests target shutdown(), and this adds wiring tests for registry contents (DBM on and off), resource release on cancel, and the cancel-event checks. test_check_gc_after_cancel is ported from postgres and is what found the reference cycles above — with the cycle collector disabled it asserts refcounting alone reclaims the check.

Review checklist (to be filled by reviewers)

Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
Add qa/required if this PR needs QA validation, or qa/skip-qa if it does not. Exactly one of the two is required.
If you need to backport this PR to another branch, you can add the backport/<branch-name> label to the PR and it will automatically open a backport PR once this one is merged

Made with Cursor

…lifecycle Register the seven DBM async jobs with the DatabaseCheck registry, replace the run_job_loop fan-out in check() with run_async_jobs(), and drop the cancel() override that blocked the Agent's unschedule thread on all seven job futures. Closing the main client and releasing the shared pool moves to shutdown(), and each job's cancel() override becomes shutdown() so cancel() is signal-only. Co-authored-by: Cursor <cursoragent@cursor.com>

Co-authored-by: Cursor <cursoragent@cursor.com>

cit-pr-commenter-54b7da · 2026-08-20T14:35:08Z

evalya-impact-summary

evalya impact analysis

Impact analysis: RUN-ALL — every test task will run
Trigger:         empty diff (default branch, scheduled run, or shallow-clone fallback)
Test tasks:      0 (all selected)
Publish tasks:   1 (always emitted)
Diff:            empty (no diff information)

Learn more about CI impact filtering

datadog-datadog-prod-us1 · 2026-08-20T14:38:03Z

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
• Patch Coverage: 97.71%
• Overall Coverage: 94.31%

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: c31d4f3 | Docs | View more details | Give us feedback!}

Each job's shutdown() now drops its _check reference alongside closing its client, so cancel() leaves nothing pinning the check. ClickhouseMetadata also drops the schema collector and ClickhouseQueryCompletions the explain plans helper; both hold the check, and the latter also holds a bound method of its job. The check's own shutdown() releases the query manager and the health reporter. test_check_gc_after_cancel, ported from postgres, is what found these: with the cycle collector disabled it asserts refcounting alone reclaims the check. Co-authored-by: Cursor <cursoragent@cursor.com>

The query samples, table metrics, and parts and merges jobs never consulted the cancel event, so a cancel landing mid-tick still issued every remaining query in that tick. Each one blocks until the client read_timeout elapses, delaying the Agent's unschedule by a timeout per query. Check the cancel event before issuing each query, matching the convention already used by the query log job and by postgres. In the two samples methods the check sits above the broad except that would otherwise swallow it, so the cancellation aborts the whole collection rather than continuing to the next query. Co-authored-by: Cursor <cursoragent@cursor.com>

Rename _init_dbm_components to _register_async_jobs and adopt the shape the other DBM integrations use: the job attributes default to None in __init__, the shared dbm gate is a single early return instead of a conjunct repeated seven times, and the per-job else branches that only reassigned None are gone. Pure refactor, no change to which jobs are built or to the attributes callers see when a job is disabled. Co-authored-by: Cursor <cursoragent@cursor.com>

Each job keeps a one-line comment naming its feature and source tables. Co-authored-by: Cursor <cursoragent@cursor.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 23071d26be

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

eric-weaver · 2026-08-21T15:14:26Z

/codex

eric-weaver · 2026-08-21T15:15:11Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 48954b49a4

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-08-21T15:19:31Z

-        # Run parts and merges monitoring if enabled
-        if self.parts_and_merges:
-            self.parts_and_merges.run_job_loop(self.tags)
+        self.run_async_jobs(self.tags)


Stop core queries once the check is cancelled

When cancellation arrives before this line—for example during _query_manager.execute()—run_async_jobs() is the first cancellation-aware operation in check(). The inherited DatabaseCheck.cancel() now intentionally leaves _client open until check() returns, while QueryManager.execute() continues through every configured query, so a cancelled check can start additional core queries and remain alive for multiple read_timeout periods. Poll is_cancelled between the main collection stages or make the query executor abort before starting each subsequent query.

Useful? React with 👍 / 👎.

This is currently expected

The Agent cancels from another thread, and the inherited lifecycle defers teardown until check() returns so the main client cannot be closed mid-run. Nothing else interrupted the run, so a cancelled check worked through cluster resolution and every configured core query, each bounded only by the client read timeout. Gate the stages that query on is_cancelled. The previous cancel() override aborted these queries by closing the client out from under them, which is the crash the base protocol defers teardown to avoid. Co-authored-by: Cursor <cursoragent@cursor.com>

This reverts commit 3560f5a. The inherited lifecycle already handles this. cancel() sets the flag, signals the jobs and returns without waiting when the check is mid-run, so the Agent's unschedule thread is never blocked by core collection. The deferred _finalize() then runs from run()'s finally on the check's own thread, which is also what keeps the main client open for the rest of the run. That leaves letting the in-flight run finish, which costs at most one collection and yields one complete set of metrics instead of a partial one. If we do want to interrupt core collection, QueryExecutor.execute() in datadog_checks_base is the place, since it loops every compiled query with no cancellation hook and would cover every integration rather than just this one. Co-authored-by: Cursor <cursoragent@cursor.com>

dd-octo-sts · 2026-08-21T15:56:43Z

Validation Report

All 21 validations passed.

Show details

Validation	Description	Status
`agent-reqs`	Verify check versions match the Agent requirements file	✅
`ci`	Validate CI configuration and code coverage settings	✅
`codeowners`	Validate every integration has a CODEOWNERS entry	✅
`config`	Validate default configuration files against spec.yaml	✅
`dep`	Verify dependency pins are consistent and Agent-compatible	✅
`http`	Validate integrations use the HTTP wrapper correctly	✅
`imports`	Validate check imports do not use deprecated modules	✅
`integration-style`	Validate check code style conventions	✅
`jmx-metrics`	Validate JMX metrics definition files and config	✅
`labeler`	Validate PR labeler config matches integration directories	✅
`legacy-signature`	Validate no integration uses the legacy Agent check signature	✅
`license-headers`	Validate Python files have proper license headers	✅
`licenses`	Validate third-party license attribution list	✅
`metadata`	Validate metadata.csv metric definitions	✅
`models`	Validate configuration data models match spec.yaml	✅
`openmetrics`	Validate OpenMetrics integrations disable the metric limit	✅
`package`	Validate Python package metadata and naming	✅
`qa-label`	Validate the pull request declares whether it needs QA for the next Agent release	✅
`readmes`	Validate README files have required sections	✅
`saved-views`	Validate saved view JSON file structure and fields	✅
`version`	Validate version consistency between package and changelog	✅

View full run

temporal-github-worker-1 Bot added agent/review-requested ecosystems/review-requested product/review-requested labels Aug 20, 2026

dd-octo-sts Bot added the integration/clickhouse label Aug 20, 2026

eric-weaver added the qa/required QA is required for this PR and will generate a QA card label Aug 20, 2026

Add changelog

be8f1d2

Co-authored-by: Cursor <cursoragent@cursor.com>

eric-weaver mentioned this pull request Aug 20, 2026

[DBMON-6881] Move mysql onto the DatabaseCheck job registry and cancellation lifecycle #24936

Merged

3 tasks

eric-weaver and others added 3 commits August 20, 2026 13:00

eric-weaver commented Aug 20, 2026

View reviewed changes

Comment thread clickhouse/changelog.d/24934.fixed Outdated

eric-weaver and others added 2 commits August 20, 2026 16:11

Update clickhouse/changelog.d/24934.fixed

989d0cd

Trim the comments in clickhouse's job registration

23071d2

Each job keeps a one-line comment naming its feature and source tables. Co-authored-by: Cursor <cursoragent@cursor.com>

eric-weaver marked this pull request as ready for review August 20, 2026 20:25

eric-weaver requested review from a team as code owners August 20, 2026 20:25

dd-octo-sts Bot added team/agent-integrations team/database-monitoring-agent labels Aug 20, 2026

chatgpt-codex-connector Bot reviewed Aug 20, 2026

View reviewed changes

Comment thread clickhouse/datadog_checks/clickhouse/table_metrics.py

Comment thread clickhouse/datadog_checks/clickhouse/metadata.py Outdated

Comment thread clickhouse/tests/test_unit.py Outdated

sangeetashivaji approved these changes Aug 20, 2026

View reviewed changes

PR feedback

48954b4

chatgpt-codex-connector Bot reviewed Aug 21, 2026

View reviewed changes

eric-weaver and others added 2 commits August 21, 2026 11:36

azhou-datadog approved these changes Aug 24, 2026

View reviewed changes

eric-weaver added this pull request to the merge queue Aug 24, 2026

Merged via the queue into master with commit a2a4047 Aug 24, 2026
59 checks passed

eric-weaver deleted the eric.weaver/DBMON-6881-clickhouse branch August 24, 2026 17:35

dd-octo-sts Bot added this to the 7.83.0 milestone Aug 24, 2026

Navigation Menu

Uh oh!

[DBMON-6881] Move clickhouse onto the DatabaseCheck job registry and cancellation lifecycle - #24934

[DBMON-6881] Move clickhouse onto the DatabaseCheck job registry and cancellation lifecycle#24934
eric-weaver merged 10 commits into
masterfrom
eric.weaver/DBMON-6881-clickhouse

eric-weaver commented Aug 20, 2026 •

edited

Loading

Uh oh!

cit-pr-commenter-54b7da Bot commented Aug 20, 2026 •

edited

Loading

Uh oh!

datadog-datadog-prod-us1 Bot commented Aug 20, 2026 •

edited

Loading

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eric-weaver commented Aug 21, 2026

Uh oh!

eric-weaver commented Aug 21, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Aug 21, 2026

Uh oh!

eric-weaver Aug 21, 2026

Uh oh!

dd-octo-sts Bot commented Aug 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

eric-weaver commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Motivation

Review checklist (to be filled by reviewers)

Uh oh!

cit-pr-commenter-54b7da Bot commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

evalya-impact-summary

Uh oh!

datadog-datadog-prod-us1 Bot commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eric-weaver commented Aug 21, 2026

Uh oh!

eric-weaver commented Aug 21, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

eric-weaver Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

dd-octo-sts Bot commented Aug 21, 2026

Validation Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

eric-weaver commented Aug 20, 2026 •

edited

Loading

cit-pr-commenter-54b7da Bot commented Aug 20, 2026 •

edited

Loading

datadog-datadog-prod-us1 Bot commented Aug 20, 2026 •

edited

Loading