Move MySQL schema collection to the shared SchemaCollector and remove the legacy collector - #24653
Move MySQL schema collection to the shared SchemaCollector and remove the legacy collector#24653eric-weaver wants to merge 8 commits into
Conversation
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: fe08b50 | Docs | Datadog PR Page | Give us feedback! |
Add a MySqlSchemaCollector that streams schema metadata in chunks via the shared SchemaCollector base class. It supports a single JSON-aggregation query per database (default) and a chunked fallback, gated by version with a hidden use_legacy_collection escape hatch back to the legacy DatabasesData path. Co-authored-by: Cursor <cursoragent@cursor.com>
d838b35 to
b294cc7
Compare
Co-authored-by: Cursor <cursoragent@cursor.com>
…lector Unify schema collection on the shared SchemaCollector so every path emits the collection_payloads_count snapshot markers. The strategy is now selected by server version: single_query on JSON-capable servers (MySQL >= 5.7.22 / MariaDB >= 10.5.0) and chunked on older versions, which works everywhere. A hidden collection_strategy option can still force either strategy. Delete the legacy DatabasesData collector and its use_legacy_collection escape hatch, which could not emit snapshot markers, along with the legacy-only unit tests and the time-based truncation test. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7eeeb9d8ce
ℹ️ 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".
| if self._effective_strategy() == STRATEGY_CHUNKED: | ||
| yield _ChunkedTableCursor(self._iter_chunked_tables(database_name)) | ||
| return |
There was a problem hiding this comment.
Preserve max_execution_time for chunked collection
When the automatic fallback selects chunked on older MySQL/MariaDB versions, or when collection_strategy: chunked is forced, this branch returns before applying any SQL timeout/deadline; _iter_chunked_tables() then runs the table query plus all detail queries without checking self._config.max_execution_time. That means large or slow schemas can run far beyond the documented collect_schemas.max_execution_time limit, unlike the single-query path and the legacy collector's truncation behavior.
Useful? React with 👍 / 👎.
Validation ReportAll 21 validations passed. Show details
|
What does this PR do?
Reworks MySQL schema collection onto the shared
SchemaCollectorbase class so everycollection path streams schema metadata in chunks and emits the
collection_payloads_countsnapshot markers the backend uses to detect complete point-in-time snapshots.
MySqlSchemaCollector(datadog_checks/mysql/schemas.py) with two strategies thatproduce byte-identical payloads:
single_query: one JSON-aggregation query per database, streamed one row (table) at atime via an unbuffered server-side cursor.
chunked: streams the table list and fetches column / index / foreign-key / partitiondetail per chunk with flat
INFORMATION_SCHEMAqueries.single_queryon JSON-capableservers (MySQL >= 5.7.22 / MariaDB >= 10.5.0) and
chunkedon older versions (works onevery supported version). A hidden
collection_strategyoption can force either strategyfor debugging.
DatabasesDatacollector, which used buffered cursors and could not emitsnapshot markers, so keeping it would leave older servers without consistent snapshots.
Behavior change to note: the legacy collector, on exceeding
max_execution_time, emitted apartial payload plus a
collection_errors: [{error_type: "truncated"}]marker. The sharedcollector enforces
max_execution_timeat the SQL level; if a database's query exceeds it,that database is skipped (already-flushed chunk payloads are retained) and no
truncatedmarker is emitted.
Motivation
The legacy collector issues
1 + 4 x ceil(tables/500)round trips per database and buffersresults, so both round trips and client memory scale with table count. The
single_querypath streams unbuffered and issues ~1 query per database.
Benchmarks below drive the real collector code against a live server via a minimal fake check,
capturing payloads in-process (synthetic schema: 12 columns/table, a PK, two secondary indexes,
a foreign key to a sibling table, range partitions on every 10th table).
Scaling table count (MySQL 8.0.46, localhost)
peak memis the Pythontracemallocpeak during collection (emitted payloads discarded so thefigure reflects the collector, not the harness).
single_querystays flat at ~12 MiB from 5k to 20k tables (row-at-a-timestreaming, per-table aggregation done server-side). Legacy nearly doubles (20.5 -> 39.9 MiB);
chunkedsits in between (~18 MiB).single_queryissues ~1 query per database (11 for 10 dbs); legacy andchunkedscale with table count (171 at 20k tables).single_queryis fastest even on localhost (~17-22% faster than legacy), and thegap grows with network latency (below).
single_queryandchunkedare byte-identical at every scale.Database-side impact (5,000 tables, 5 dbs, MySQL 8.0.46)
Measured via session-scoped
SHOW SESSION STATUSdeltas around each run.single_querydoes notmake the server scan more of
information_schema.(
tmp_disk_tables = 0) at this scale.single_querysends ~2.8x more bytes because server-side JSON repeats objectkeys per row; the client normalizes this back down to a similar final payload. So it trades
higher DB egress for fewer round trips, bounded client memory, and equal server reads.
Network-latency sweep (MySQL 8.0, 5,000 tables, Toxiproxy on server->client)
single_query(6 round trips) changes little across the range (+0.06 s from 0 to 25 ms), whilechunkedandlegacy(46 round trips each) grow by ~1.2-1.5 s over the same range, reflectingthe lower round-trip count on a latency-bound link.
Cross-version payload equivalence
single_queryoutput was deep-compared against the legacy collector on every supported version(reassembling chunked payloads into per-database table maps; envelope fields ignored):
Review checklist (to be filled by reviewers)
qa/requiredif this PR needs QA validation, orqa/skip-qaif it does not. Exactly one of the two is required.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged