feat: add mysql-zstd_compression_level variable#5637
feat: add mysql-zstd_compression_level variable#5637renecannao wants to merge 5 commits intov3.0from
Conversation
📝 WalkthroughWalkthroughThe changes introduce a new thread-safe integer configuration variable Changes
Sequence DiagramsequenceDiagram
participant Admin as Admin/User
participant Config as Config Registry
participant Thread as Thread Handler
participant Protocol as Protocol Layer
participant Stream as Data Stream
Admin->>Config: SET mysql-zstd_compression_level=N
Config->>Config: Validate N in [1, ZSTD_maxCLevel()]
Admin->>Config: LOAD MYSQL VARIABLES TO RUNTIME
Config->>Thread: Update mysql_thread___zstd_compression_level
Thread->>Thread: Cache per-thread value
Protocol->>Thread: Read zstd_compression_level config
Thread-->>Protocol: Return cached value
Protocol->>Protocol: Determine ZSTD compression level<br/>(fallback chain: var → thread-local)
Protocol->>Stream: Initialize with zstd_compression_level
Stream->>Stream: Call get_zstd_compression_level()
Stream->>Stream: Read mysql_thread___zstd_compression_level
Stream->>Stream: Clamp to valid range [1, ZSTD_maxCLevel()]
Stream-->>Protocol: Return effective compression level
Protocol->>Stream: Compress data with selected level
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a dedicated mysql-zstd_compression_level variable to ProxySQL, allowing zstd compression levels (1–22) to be configured independently from zlib. The changes include the necessary thread-local variable declarations, registration within the MySQL thread system, and updates to the protocol handshake and data stream logic to utilize the new setting. Additionally, comprehensive TAP tests and design documentation have been added. Feedback on the code changes identifies a potential crash in the test suite when handling null connection pointers, redundant clamping logic in the protocol implementation, and a suggestion to avoid the MYSQL_QUERY_T macro when testing for rejected configuration values to prevent premature test failure.
| // Connect to admin | ||
| MYSQL* admin = init_mysql_conn(cl.host, cl.admin_port, cl.admin_username, cl.admin_password); | ||
| if (!admin) { | ||
| fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(admin)); |
There was a problem hiding this comment.
Calling mysql_error(admin) when admin is NULL results in undefined behavior and will likely cause a crash. Since this block is only entered if admin is NULL, you should use a generic error message or check the connection failure differently.
| (vars1.zstd_compression_level > 0 && vars1.zstd_compression_level <= ZSTD_maxCLevel()) | ||
| ? vars1.zstd_compression_level | ||
| : static_cast<uint8_t>(std::min<int>(ZSTD_maxCLevel(), std::max<int>(1, mysql_thread___protocol_compression_level))); | ||
| : static_cast<uint8_t>(std::min<int>(ZSTD_maxCLevel(), std::max<int>(1, mysql_thread___zstd_compression_level))); |
There was a problem hiding this comment.
The clamping logic using std::min and std::max is redundant here. The variable mysql_thread___zstd_compression_level is already constrained to the range [1, ZSTD_maxCLevel()] by the admin interface registration in VariablesPointers_int (see lib/MySQL_Thread.cpp).
: static_cast<uint8_t>(mysql_thread___zstd_compression_level);| } | ||
|
|
||
| // Test 5: Reject value 0 | ||
| { |
There was a problem hiding this comment.
If MYSQL_QUERY_T is defined to assert success (which is common in this test suite), these tests for rejection will fail or cause the test process to exit prematurely because SET mysql-zstd_compression_level=0 is expected to return an error from the server. To test rejection, you should explicitly check that mysql_query returns a non-zero value instead of using the MYSQL_QUERY_T macro.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.gitignore (1)
219-219: Remove duplicate.worktrees/ignore entry.Line 219 duplicates the existing rule at Line 5, so this can be dropped to keep
.gitignoreclean.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore at line 219, Remove the duplicate ".worktrees/" entry from .gitignore by deleting the redundant ".worktrees/" ignore rule (leave the original single occurrence intact) so there is only one .worktrees/ entry in the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/superpowers/plans/2026-04-16-zstd-compression-level.md`:
- Line 16: The task headings like "### Task 1: Add thread-local variable and
struct member" jump directly from the document H1 and violate markdown heading
increment rules; change these "### Task ..." headings to "## Task ..." (or
insert an intervening H2 section before the task group) so the sequence
increments correctly; apply the same fix to the other task headings mentioned
(the repeated "### Task ..." occurrences) to satisfy MD001/heading-increment.
In `@test/tap/tests/mysql-zstd_compression_level-t.cpp`:
- Around line 73-95: The test currently hardcodes 22/23 and uses MYSQL_QUERY_T
which aborts on SQL errors; instead query ProxySQL at runtime for the true upper
bound (use a SHOW VARIABLES LIKE 'mysql-zstd_compression_level' or set a
known-valid value then probe upward) and replace the hardcoded constants with
that derived max (matching ZSTD_maxCLevel()). For the rejection cases
(previously Tests 5–6 using MYSQL_QUERY_T), call mysql_query() directly and
assert it returns non-zero to prove the SET failed, then LOAD MYSQL VARIABLES TO
RUNTIME and verify get_variable_value_int(admin, "mysql-zstd_compression_level",
true) still equals the known-valid value; update references to MYSQL_QUERY_T,
mysql_query, get_variable_value_int and any probing logic to implement this
behavior.
- Around line 130-144: The test currently calls init_mysql_conn(..., false,
true) which only enables generic MYSQL_OPT_COMPRESS so the client/server may
negotiate zlib instead of zstd; update the test to either force zstd-only
negotiation or assert the negotiated algorithm before passing: after
establishing proxy_cmp, query connection info (or use client capability flags)
to confirm CLIENT_ZSTD_COMPRESSION_ALGORITHM was negotiated (or set the client
to advertise only zstd capabilities), and only then run the SELECT/ok assertion
that mysql-zstd_compression_level=3 is exercised; reference init_mysql_conn,
MYSQL_OPT_COMPRESS, CLIENT_COMPRESS, CLIENT_ZSTD_COMPRESSION_ALGORITHM,
mysql-zstd_compression_level and proxy_cmp when making the change.
---
Nitpick comments:
In @.gitignore:
- Line 219: Remove the duplicate ".worktrees/" entry from .gitignore by deleting
the redundant ".worktrees/" ignore rule (leave the original single occurrence
intact) so there is only one .worktrees/ entry in the file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dedab92e-9170-4c78-a3ba-ce7cb2502255
📒 Files selected for processing (11)
.gitignoredocs/superpowers/plans/2026-04-16-zstd-compression-level.mddocs/superpowers/specs/2026-04-16-zstd-compression-level-design.mdinclude/MySQL_Thread.hinclude/proxysql_structs.hlib/MySQL_Protocol.cpplib/MySQL_Thread.cpplib/mysql_data_stream.cpptest/tap/groups/groups.jsontest/tap/tests/mysql-zstd_compression_level-t.cpptest/tap/tests/reg_test_2793-compression-t.cpp
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{cpp,h,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{cpp,h,hpp}: Feature tiers controlled by flags:PROXYSQL31=1for v3.1.x (FFTO, TSDB),PROXYSQLGENAI=1for v4.0.x (GenAI, MCP, Anomaly Detection).PROXYSQLGENAI=1impliesPROXYSQL31=1which impliesPROXYSQLFFTO=1andPROXYSQLTSDB=1
Conditional compilation uses#ifdef PROXYSQLGENAI,#ifdef PROXYSQL31,#ifdef PROXYSQLFFTO,#ifdef PROXYSQLTSDB, and#ifdef PROXYSQLCLICKHOUSEto gate feature-specific code
Class names use PascalCase with protocol prefixes:MySQL_,PgSQL_, orProxySQL_(e.g.,MySQL_Protocol,PgSQL_Session,ProxySQL_Admin)
Member variables use snake_case naming convention
Constants and macros use UPPER_SNAKE_CASE naming convention
Require C++17 standard for all source code
Use RAII (Resource Acquisition Is Initialization) pattern for resource management in C++ code
Use jemalloc for memory allocation in C++ code
Use pthread mutexes for synchronization andstd::atomic<>for counters
Files:
include/MySQL_Thread.hlib/mysql_data_stream.cppinclude/proxysql_structs.hlib/MySQL_Thread.cpptest/tap/tests/mysql-zstd_compression_level-t.cpplib/MySQL_Protocol.cpptest/tap/tests/reg_test_2793-compression-t.cpp
include/**/*.{h,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
Include guards use the pattern
#ifndef __CLASS_*_H(e.g.,#ifndef __ADMIN_H__,#ifndef __PROXYSQL_PROTOCOL_H__)
Files:
include/MySQL_Thread.hinclude/proxysql_structs.h
lib/**/*.cpp
📄 CodeRabbit inference engine (CLAUDE.md)
One class per file is the typical convention in lib/ directory
Files:
lib/mysql_data_stream.cpplib/MySQL_Thread.cpplib/MySQL_Protocol.cpp
test/tap/tests/**/{test_*.cpp,*-t.cpp}
📄 CodeRabbit inference engine (CLAUDE.md)
Test files follow naming pattern
test_*.cppor*-t.cppin test/tap/tests/
Files:
test/tap/tests/mysql-zstd_compression_level-t.cpptest/tap/tests/reg_test_2793-compression-t.cpp
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: sysown/proxysql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-12T11:27:41.156Z
Learning: Applies to **/*.{cpp,h,hpp} : Feature tiers controlled by flags: `PROXYSQL31=1` for v3.1.x (FFTO, TSDB), `PROXYSQLGENAI=1` for v4.0.x (GenAI, MCP, Anomaly Detection). `PROXYSQLGENAI=1` implies `PROXYSQL31=1` which implies `PROXYSQLFFTO=1` and `PROXYSQLTSDB=1`
📚 Learning: 2026-04-12T11:27:41.156Z
Learnt from: CR
Repo: sysown/proxysql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-12T11:27:41.156Z
Learning: Applies to test/tap/tests/**/{test_*.cpp,*-t.cpp} : Test files follow naming pattern `test_*.cpp` or `*-t.cpp` in test/tap/tests/
Applied to files:
.gitignoretest/tap/tests/mysql-zstd_compression_level-t.cpp
📚 Learning: 2026-04-01T21:27:03.216Z
Learnt from: wazir-ahmed
Repo: sysown/proxysql PR: 5557
File: test/tap/tests/unit/gtid_set_unit-t.cpp:14-17
Timestamp: 2026-04-01T21:27:03.216Z
Learning: In ProxySQL's unit test directory (test/tap/tests/unit/), test_globals.h and test_init.h are only required for tests that depend on the ProxySQL runtime globals/initialization (i.e., tests that exercise components linked against libproxysql.a). Pure data-structure or utility tests (e.g., ezoption_parser_unit-t.cpp, gtid_set_unit-t.cpp, gtid_trxid_interval_unit-t.cpp) only need tap.h and the relevant project header — omitting test_globals.h and test_init.h is correct and intentional in these cases.
Applied to files:
.gitignoretest/tap/groups/groups.jsonlib/MySQL_Thread.cpptest/tap/tests/mysql-zstd_compression_level-t.cppdocs/superpowers/plans/2026-04-16-zstd-compression-level.mdtest/tap/tests/reg_test_2793-compression-t.cpp
📚 Learning: 2026-03-26T16:38:58.553Z
Learnt from: yuji-hatakeyama
Repo: sysown/proxysql PR: 5548
File: lib/mysql_connection.cpp:1837-1843
Timestamp: 2026-03-26T16:38:58.553Z
Learning: In `lib/mysql_connection.cpp`, when reviewing `SHOW WARNINGS` handling, treat the digest source as an intentional design choice: both `update_warning_count_from_connection()` and the `add_eof()` call under `ASYNC_USE_RESULT_CONT` detect warnings using `myds->sess->CurrentQuery.QueryParserArgs.digest_text` (comment-stripped digest text). This is expected to fail/behave differently when `mysql-query_digests_keep_comment=1` (digest_text includes comments) or when `mysql-query_digests=0` (digest_text unavailable). Do not require a change unless the regression test coverage is expanded (noting `reg_test_5306-show_warnings_with_comment-t` explicitly excludes these configurations as an accepted limitation).
Applied to files:
.gitignore
📚 Learning: 2026-04-12T11:27:41.156Z
Learnt from: CR
Repo: sysown/proxysql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-12T11:27:41.156Z
Learning: Applies to **/*.{cpp,h,hpp} : Feature tiers controlled by flags: `PROXYSQL31=1` for v3.1.x (FFTO, TSDB), `PROXYSQLGENAI=1` for v4.0.x (GenAI, MCP, Anomaly Detection). `PROXYSQLGENAI=1` implies `PROXYSQL31=1` which implies `PROXYSQLFFTO=1` and `PROXYSQLTSDB=1`
Applied to files:
include/proxysql_structs.htest/tap/groups/groups.jsonlib/MySQL_Thread.cpptest/tap/tests/mysql-zstd_compression_level-t.cpplib/MySQL_Protocol.cppdocs/superpowers/plans/2026-04-16-zstd-compression-level.mdtest/tap/tests/reg_test_2793-compression-t.cppdocs/superpowers/specs/2026-04-16-zstd-compression-level-design.md
📚 Learning: 2026-04-12T11:27:41.156Z
Learnt from: CR
Repo: sysown/proxysql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-12T11:27:41.156Z
Learning: Applies to **/*.{cpp,h,hpp} : Conditional compilation uses `#ifdef PROXYSQLGENAI`, `#ifdef PROXYSQL31`, `#ifdef PROXYSQLFFTO`, `#ifdef PROXYSQLTSDB`, and `#ifdef PROXYSQLCLICKHOUSE` to gate feature-specific code
Applied to files:
include/proxysql_structs.hlib/MySQL_Thread.cpp
📚 Learning: 2026-03-26T16:39:02.446Z
Learnt from: yuji-hatakeyama
Repo: sysown/proxysql PR: 5548
File: lib/mysql_connection.cpp:1837-1843
Timestamp: 2026-03-26T16:39:02.446Z
Learning: In ProxySQL's lib/mysql_connection.cpp, `SHOW WARNINGS` detection for both `update_warning_count_from_connection()` and the `add_eof()` call in `ASYNC_USE_RESULT_CONT` intentionally uses `myds->sess->CurrentQuery.QueryParserArgs.digest_text` (comment-stripped digest text). This means the fix/feature does not work when `mysql-query_digests_keep_comment=1` (digest_text contains comments) or `mysql-query_digests=0` (digest_text is unavailable) — these configurations are explicitly excluded from the regression test for `reg_test_5306-show_warnings_with_comment-t`. This design is consistent across the codebase and is an accepted, documented limitation.
Applied to files:
test/tap/groups/groups.json
📚 Learning: 2026-04-12T11:27:41.156Z
Learnt from: CR
Repo: sysown/proxysql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-12T11:27:41.156Z
Learning: Applies to test/tap/tests/unit/**/*.cpp : Unit tests must use `test_globals.h` and `test_init.h` header files and link against `libproxysql.a` via custom test harness
Applied to files:
test/tap/tests/mysql-zstd_compression_level-t.cpptest/tap/tests/reg_test_2793-compression-t.cpp
📚 Learning: 2026-01-20T09:34:19.124Z
Learnt from: yuji-hatakeyama
Repo: sysown/proxysql PR: 5307
File: test/tap/tests/reg_test_5306-show_warnings_with_comment-t.cpp:39-48
Timestamp: 2026-01-20T09:34:19.124Z
Learning: In ProxySQL's TAP test suite, resource leaks (e.g., not calling mysql_close() on early return paths) are commonly tolerated because test processes are short-lived and OS frees resources on exit. This pattern applies to all C++ test files under test/tap/tests. When reviewing, recognize this as a project-wide test convention and focus on test correctness and isolation rather than insisting on fixing such leaks in these test files.
Applied to files:
test/tap/tests/mysql-zstd_compression_level-t.cpptest/tap/tests/reg_test_2793-compression-t.cpp
📚 Learning: 2026-04-12T11:27:41.156Z
Learnt from: CR
Repo: sysown/proxysql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-12T11:27:41.156Z
Learning: Applies to **/*.{cpp,h,hpp} : Class names use PascalCase with protocol prefixes: `MySQL_`, `PgSQL_`, or `ProxySQL_` (e.g., `MySQL_Protocol`, `PgSQL_Session`, `ProxySQL_Admin`)
Applied to files:
lib/MySQL_Protocol.cpp
📚 Learning: 2026-04-11T13:17:55.508Z
Learnt from: renecannao
Repo: sysown/proxysql PR: 5607
File: doc/GH-Actions/README.md:13-18
Timestamp: 2026-04-11T13:17:55.508Z
Learning: When using GitHub-flavored Markdown headings, be aware that an em-dash surrounded by spaces (written as ` — `) affects the generated anchor/slug: GitHub replaces spaces with hyphens and removes non-alphanumeric punctuation, which can produce double hyphens (e.g., `## Foo — bar` → anchor `#foo--bar`, not `#foo-bar`). If you reference these anchors (e.g., internal links), ensure the expected slug matches this behavior.
Applied to files:
docs/superpowers/plans/2026-04-16-zstd-compression-level.mddocs/superpowers/specs/2026-04-16-zstd-compression-level-design.md
📚 Learning: 2026-04-11T13:17:55.509Z
Learnt from: renecannao
Repo: sysown/proxysql PR: 5607
File: doc/GH-Actions/README.md:13-18
Timestamp: 2026-04-11T13:17:55.509Z
Learning: When reviewing GitHub-flavored Markdown links/anchors, remember that heading-to-anchor slug generation treats spaces as hyphens and removes punctuation. If a heading contains an em-dash surrounded by spaces (e.g. ` — `), the slugs can legitimately include a double hyphen where the two surrounding space-runs become `-` on either side of the removed em-dash (e.g. `...vocabulary--read...`). Do not flag double-hyphens in anchor links for em-dash-containing headings as errors; they reflect GitHub’s correct slug behavior.
Applied to files:
docs/superpowers/plans/2026-04-16-zstd-compression-level.mddocs/superpowers/specs/2026-04-16-zstd-compression-level-design.md
🪛 Clang (14.0.6)
test/tap/tests/mysql-zstd_compression_level-t.cpp
[error] 12-12: 'cstdio' file not found
(clang-diagnostic-error)
🪛 Cppcheck (2.20.0)
lib/mysql_data_stream.cpp
[warning] 32-32: If memory allocation fails, then there is a possible null pointer dereference
(nullPointerOutOfMemory)
🪛 LanguageTool
docs/superpowers/specs/2026-04-16-zstd-compression-level-design.md
[style] ~133-~133: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...in mysql_servers / pgsql_servers. - No per-server or per-user zstd level confi...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.22.0)
docs/superpowers/plans/2026-04-16-zstd-compression-level.md
[warning] 16-16: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
docs/superpowers/specs/2026-04-16-zstd-compression-level-design.md
[warning] 16-16: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🔇 Additional comments (9)
.gitignore (1)
214-215: Good ignore rule for unit test binaries.This addition is useful and aligns with keeping local test artifacts out of version control.
lib/mysql_data_stream.cpp (1)
32-33: Correct fallback switch to dedicated zstd thread variable.The fallback now correctly uses
mysql_thread___zstd_compression_levelwhile preserving range validation.include/MySQL_Thread.h (1)
491-491:zstd_compression_levelfield addition looks correct.This cleanly extends thread variables for separate zstd-level management.
lib/MySQL_Protocol.cpp (1)
2303-2303: Handshake fallback now correctly references the zstd-specific setting.This keeps zstd negotiation independent from zlib configuration as intended.
test/tap/tests/reg_test_2793-compression-t.cpp (1)
37-52: Nice coverage addition for admin visibility/default value.The new check is practical and complements the compression behavior assertions well.
include/proxysql_structs.h (2)
1299-1299: Looks good: new TLS zstd variable is declared consistently.The added
mysql_thread___zstd_compression_leveldefinition matches existing thread-local variable patterns and naming conventions.
1633-1633: Extern declaration is correctly mirrored.Good parity with the
PROXYSQL_EXTERNdefinition branch formysql_thread___zstd_compression_level.test/tap/groups/groups.json (1)
90-90: Test group registration looks correct.The new
mysql-zstd_compression_level-tentry is wired with the expected MySQL TAP group variants.test/tap/tests/mysql-zstd_compression_level-t.cpp (1)
47-50:⚠️ Potential issue | 🟠 MajorAvoid calling
mysql_error()on a null admin handle.When
init_mysql_conn()fails,adminis null here, so the error-reporting path can crash before the test reports the actual setup failure.Suggested fix
MYSQL* admin = init_mysql_conn(cl.host, cl.admin_port, cl.admin_username, cl.admin_password); if (!admin) { - fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(admin)); + fprintf(stderr, "File %s, line %d, Error: failed to connect to admin\n", __FILE__, __LINE__); return EXIT_FAILURE; }⛔ Skipped due to learnings
Learnt from: yuji-hatakeyama Repo: sysown/proxysql PR: 5307 File: test/tap/tests/reg_test_5306-show_warnings_with_comment-t.cpp:24-28 Timestamp: 2026-01-20T07:40:34.938Z Learning: In ProxySQL test files, calling `mysql_error(NULL)` after `mysql_init()` failure is safe because the MariaDB client library implementation returns an empty string for NULL handles (not undefined behavior).Learnt from: yuji-hatakeyama Repo: sysown/proxysql PR: 5307 File: test/tap/tests/reg_test_5306-show_warnings_with_comment-t.cpp:39-48 Timestamp: 2026-01-20T09:34:27.165Z Learning: In ProxySQL test files (test/tap/tests/), resource leaks (such as not calling `mysql_close()` on early return paths) are not typically fixed because test processes are short-lived and the OS frees resources on process exit. This is a common pattern across the test suite.Learnt from: wazir-ahmed Repo: sysown/proxysql PR: 5557 File: test/tap/tests/unit/gtid_set_unit-t.cpp:14-17 Timestamp: 2026-04-01T21:27:03.216Z Learning: In ProxySQL's unit test directory (test/tap/tests/unit/), test_globals.h and test_init.h are only required for tests that depend on the ProxySQL runtime globals/initialization (i.e., tests that exercise components linked against libproxysql.a). Pure data-structure or utility tests (e.g., ezoption_parser_unit-t.cpp, gtid_set_unit-t.cpp, gtid_trxid_interval_unit-t.cpp) only need tap.h and the relevant project header — omitting test_globals.h and test_init.h is correct and intentional in these cases.Learnt from: CR Repo: sysown/proxysql PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-04-12T11:27:41.156Z Learning: Applies to test/tap/tests/unit/**/*.cpp : Unit tests must use `test_globals.h` and `test_init.h` header files and link against `libproxysql.a` via custom test harness
|
|
||
| --- | ||
|
|
||
| ### Task 1: Add thread-local variable and struct member |
There was a problem hiding this comment.
Fix the heading-level jump in the task sections.
These ### Task ... headings jump straight from the file’s H1, which trips MD001/heading-increment. Make the task headings ## or add an intervening H2 section so markdownlint passes cleanly.
Also applies to: 63-63, 145-145, 224-224, 408-408, 440-440, 548-548
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 16-16: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/superpowers/plans/2026-04-16-zstd-compression-level.md` at line 16, The
task headings like "### Task 1: Add thread-local variable and struct member"
jump directly from the document H1 and violate markdown heading increment rules;
change these "### Task ..." headings to "## Task ..." (or insert an intervening
H2 section before the task group) so the sequence increments correctly; apply
the same fix to the other task headings mentioned (the repeated "### Task ..."
occurrences) to satisfy MD001/heading-increment.
| // Test 10: Functional test - zstd compressed connection works | ||
| { | ||
| MYSQL_QUERY_T(admin, "SET mysql-zstd_compression_level=3"); | ||
| MYSQL_QUERY_T(admin, "LOAD MYSQL VARIABLES TO RUNTIME"); | ||
|
|
||
| // Connect with compression enabled; this exercises zstd if the client supports it | ||
| MYSQL* proxy_cmp = init_mysql_conn(cl.host, cl.port, cl.username, cl.password, false, true); | ||
| if (proxy_cmp) { | ||
| int rc = mysql_query(proxy_cmp, "SELECT 1"); | ||
| ok(rc == 0, "Compressed connection works with zstd_compression_level=3"); | ||
| mysql_close(proxy_cmp); | ||
| } else { | ||
| diag("Skipping compressed connection test - connection failed"); | ||
| ok(true, "Compressed connection test skipped (connection failed)"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the test file
fd -t f "mysql-zstd_compression_level-t.cpp" -x cat -n {}Repository: sysown/proxysql
Length of output: 6394
🏁 Script executed:
# Look for the init_mysql_conn function definition
rg -A 10 "^[^/]*\binit_mysql_conn\s*\(" test/tap/tests/ -t cpp | head -100Repository: sysown/proxysql
Length of output: 8512
🏁 Script executed:
# Check what configuration is set in the test
rg "mysql-zstd_compression_level" test/tap/tests/mysql-zstd_compression_level-t.cpp -A 3 -B 3Repository: sysown/proxysql
Length of output: 3743
🏁 Script executed:
# Search for how compression algorithm negotiation is handled
rg "ZSTD|zstd|compression" test/tap/tests/mysql-zstd_compression_level-t.cpp -B 2 -A 2Repository: sysown/proxysql
Length of output: 4323
🏁 Script executed:
# Check if there are any related tests or documentation about compression validation
fd -t f "*compression*" test/tap/tests/ | head -20Repository: sysown/proxysql
Length of output: 351
🏁 Script executed:
# Find init_mysql_conn function definition
rg -A 20 "^[^/]*\binit_mysql_conn\s*\(" test/tap/tests/ -t cpp | grep -A 20 "^[^-]*MYSQL\*.*init_mysql_conn"Repository: sysown/proxysql
Length of output: 41
🏁 Script executed:
# Look for init_mysql_conn in header or utility files
fd -t f "utils.h" -o "utils.cpp" | xargs grep -A 15 "init_mysql_conn"Repository: sysown/proxysql
Length of output: 198
🏁 Script executed:
# Search for compression negotiation or algorithm detection in ProxySQL source
rg "ZSTD|compression.*algorithm|negotiate.*compress" lib/ -t cpp -B 2 -A 2 | head -50Repository: sysown/proxysql
Length of output: 2999
🏁 Script executed:
# Check the similar compression test to see how it validates the algorithm
cat test/tap/tests/mysql-protocol_compression_level-t.cpp | head -150Repository: sysown/proxysql
Length of output: 4982
🏁 Script executed:
# Look for any compression algorithm detection/validation mechanism
rg "mysql_get_option.*MYSQL_OPT_COMPRESSION_ALGORITHM|MYSQL_OPT_COMPRESSION_ALGORITHM" . -t cppRepository: sysown/proxysql
Length of output: 41
🏁 Script executed:
# Find utils files in test/tap/tests
fd -t f "utils" test/tap/tests/ --extension h --extension cppRepository: sysown/proxysql
Length of output: 225
🏁 Script executed:
# Search for init_mysql_conn definition more broadly
rg "init_mysql_conn" test/tap/ -B 2 -A 12 -t cpp | grep -A 12 "^[^-]*MYSQL\*.*init_mysql_conn\s*("Repository: sysown/proxysql
Length of output: 1586
🏁 Script executed:
# Check tap.h for helper definitions
cat test/tap/tests/tap.h | head -200Repository: sysown/proxysql
Length of output: 111
🏁 Script executed:
# Look for the full function signature in header files
fd -t f "\.h" test/tap/tests/ -x grep -l "init_mysql_conn" {}Repository: sysown/proxysql
Length of output: 41
🏁 Script executed:
# Get the full init_mysql_conn implementation
cat test/tap/tap/utils.cpp | grep -A 40 "^MYSQL\* init_mysql_conn"Repository: sysown/proxysql
Length of output: 1087
🏁 Script executed:
# Search for MYSQL_OPT_COMPRESSION_ALGORITHM usage in the codebase
rg "MYSQL_OPT_COMPRESSION_ALGORITHM|COMPRESSION_ALGORITHM" . -t cpp -t h -B 2 -A 2Repository: sysown/proxysql
Length of output: 4953
🏁 Script executed:
# Check if there are other compression-related options being used
rg "mysql_options.*MYSQL_OPT" test/tap/ -t cpp | head -20Repository: sysown/proxysql
Length of output: 2123
🏁 Script executed:
# Look for how to specify compression algorithm in connection setup
rg "COMPRESSION_ALGORITHM|compression.*algorithm" lib/ -t cpp -B 3 -A 3 | head -60Repository: sysown/proxysql
Length of output: 4210
🏁 Script executed:
# Check if there's documentation or comments about zstd vs zlib negotiation
rg "zstd|zlib" lib/mysql_connection.cpp -B 2 -A 2 | head -80Repository: sysown/proxysql
Length of output: 693
🏁 Script executed:
# Check for MYSQL_OPT_COMPRESSION_ALGORITHM or similar constants
rg "MYSQL_OPT_COMPRESSION" . -t cpp -t hRepository: sysown/proxysql
Length of output: 41
🏁 Script executed:
# Look at the mysql-protocol_compression_level test to see how it validates compression
grep -A 50 "Compressed connection\|compression.*algorithm\|verify.*compression\|negotiat" test/tap/tests/mysql-protocol_compression_level-t.cppRepository: sysown/proxysql
Length of output: 41
🏁 Script executed:
# Check ProxySQL documentation or comments about zstd negotiation preference
rg "prefer.*zstd|default.*compression|zstd.*default" lib/ -t cpp -B 2 -A 2Repository: sysown/proxysql
Length of output: 41
🏁 Script executed:
# Look for any configuration that sets compression defaults
rg "compression_zstd|zstd_compression" lib/mysql_connection.cpp -B 3 -A 3 | head -60Repository: sysown/proxysql
Length of output: 808
🏁 Script executed:
# Check if ProxySQL has any preference or default for zstd vs zlib negotiation
rg "CLIENT_COMPRESS|CLIENT_ZSTD" lib/MySQL_Protocol.cpp -B 3 -A 3 | head -80Repository: sysown/proxysql
Length of output: 1555
🏁 Script executed:
# Look for where ProxySQL sends its capabilities to client during handshake
rg "server_capabilities.*=" lib/MySQL_Protocol.cpp | head -20Repository: sysown/proxysql
Length of output: 1037
🏁 Script executed:
# Check if there's conditional logic that might prefer zstd
rg "use_zstd_compression\|compression_zstd\|ZSTD.*prefer" lib/ -t cpp -B 2 -A 2 | head -80Repository: sysown/proxysql
Length of output: 41
🏁 Script executed:
# Verify what flags ProxySQL advertises and in what order
grep -n "CLIENT_COMPRESS\|CLIENT_ZSTD" lib/MySQL_Protocol.cpp | head -30Repository: sysown/proxysql
Length of output: 691
🏁 Script executed:
# Check if MariaDB/MySQL client supports specifying compression algorithm
rg "MYSQL_OPT.*ZSTD|OPT.*ALGORITHM" . -t h -t cppRepository: sysown/proxysql
Length of output: 41
🏁 Script executed:
# Look for how the backend/server decides zstd negotiation
rg "vars1.use_zstd_compression\|zstd_compression.*true\|compression.*=.*true" lib/MySQL_Protocol.cpp -B 5 -A 5Repository: sysown/proxysql
Length of output: 41
🏁 Script executed:
# Check where vars1.use_zstd_compression gets set from client capabilities
rg "use_zstd_compression\s*=" lib/MySQL_Protocol.cpp -B 10 -A 2 | head -50Repository: sysown/proxysql
Length of output: 1230
🏁 Script executed:
# Verify if the client sending MYSQL_OPT_COMPRESS alone sets zstd or if it defaults to zlib
rg "CLIENT_COMPRESS\|CLIENT_ZSTD" lib/MySQL_Protocol.cpp -n | grep -E "(2291|2297)"Repository: sysown/proxysql
Length of output: 41
🏁 Script executed:
# Look at the complete logic for zstd vs zlib decision to understand negotiation
sed -n '2285,2310p' lib/MySQL_Protocol.cppRepository: sysown/proxysql
Length of output: 1153
🏁 Script executed:
# Confirm the handshake logic - check where vars1.use_zstd_compression gets set initially
rg "vars1\.use_zstd_compression\s*=" lib/MySQL_Protocol.cpp -B 10 -A 2 | head -60Repository: sysown/proxysql
Length of output: 485
🏁 Script executed:
# Look at the complete handshake parsing to understand zstd negotiation
sed -n '1770,1810p' lib/MySQL_Protocol.cppRepository: sysown/proxysql
Length of output: 1982
🏁 Script executed:
# Verify the MariaDB client library behavior - check if MYSQL_OPT_COMPRESS sets both flags
rg "MYSQL_OPT_COMPRESS" test/tap/ -B 3 -A 3 | head -40Repository: sysown/proxysql
Length of output: 1273
Test 10 does not verify that zstd compression is actually negotiated.
init_mysql_conn(..., false, true) requests generic compression via MYSQL_OPT_COMPRESS, but this allows the client and server to negotiate either zlib or zstd. If the client library sends only CLIENT_COMPRESS (without CLIENT_ZSTD_COMPRESSION_ALGORITHM capability), ProxySQL falls back to zlib compression. The test would then pass without ever reading mysql-zstd_compression_level, defeating the regression test purpose.
Verify which algorithm was negotiated (e.g., via connection info query) or force zstd-only negotiation to ensure the variable is actually exercised.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/tap/tests/mysql-zstd_compression_level-t.cpp` around lines 130 - 144,
The test currently calls init_mysql_conn(..., false, true) which only enables
generic MYSQL_OPT_COMPRESS so the client/server may negotiate zlib instead of
zstd; update the test to either force zstd-only negotiation or assert the
negotiated algorithm before passing: after establishing proxy_cmp, query
connection info (or use client capability flags) to confirm
CLIENT_ZSTD_COMPRESSION_ALGORITHM was negotiated (or set the client to advertise
only zstd capabilities), and only then run the SELECT/ok assertion that
mysql-zstd_compression_level=3 is exercised; reference init_mysql_conn,
MYSQL_OPT_COMPRESS, CLIENT_COMPRESS, CLIENT_ZSTD_COMPRESSION_ALGORITHM,
mysql-zstd_compression_level and proxy_cmp when making the change.
renecannao
force-pushed
the
v3.0_new_zstd
branch
from
2a47755 to
b3b2056
Compare
Replace mysql_thread___protocol_compression_level with mysql_thread___zstd_compression_level in both the handshake (MySQL_Protocol.cpp) and data stream (mysql_data_stream.cpp) zstd level selection paths, so the two compression algorithms can be configured independently.
- New test mysql-zstd_compression_level-t.cpp: validates default value, range enforcement (1-22), rejection of 0 and 23, independence from mysql-protocol_compression_level, and functional compressed connection. - Register mysql-zstd_compression_level-t in groups.json (same groups as mysql-protocol_compression_level-t). - Add variable visibility check to reg_test_2793-compression-t.cpp. - Add unit test binary pattern to .gitignore.
renecannao
force-pushed
the
v3.0_new_zstd
branch
from
610bf03 to
4631b1f
Compare
|
There was a problem hiding this comment.
♻️ Duplicate comments (2)
test/tap/tests/mysql-zstd_compression_level-t.cpp (2)
73-79:⚠️ Potential issue | 🟠 MajorBoundary and rejection checks are still brittle and can mask the invalid-path behavior.
Line 75/78 and Line 92/95 hardcode
22/23, while runtime code usesZSTD_maxCLevel(). Also, Line 83 and Line 92 useMYSQL_QUERY_Tfor expected-fail statements, so failure behavior isn’t asserted explicitly.Suggested test hardening
+#include <zstd.h> ... - // Test 4: Set to max (22) and load to runtime + const int zstd_max = ZSTD_maxCLevel(); + // Test 4: Set to max and load to runtime { - MYSQL_QUERY_T(admin, "SET mysql-zstd_compression_level=22"); + const string q = "SET mysql-zstd_compression_level=" + std::to_string(zstd_max); + MYSQL_QUERY_T(admin, q.c_str()); MYSQL_QUERY_T(admin, "LOAD MYSQL VARIABLES TO RUNTIME"); int rt_val = get_variable_value_int(admin, "mysql-zstd_compression_level", true); - ok(rt_val == 22, "Runtime value after SET=22 is 22, got: %d", rt_val); + ok(rt_val == zstd_max, "Runtime value after max SET is %d, got: %d", zstd_max, rt_val); } // Test 5: Reject value 0 { - MYSQL_QUERY_T(admin, "SET mysql-zstd_compression_level=0"); + int rc = mysql_query(admin, "SET mysql-zstd_compression_level=0"); + ok(rc != 0, "SET mysql-zstd_compression_level=0 is rejected"); MYSQL_QUERY_T(admin, "LOAD MYSQL VARIABLES TO RUNTIME"); - // The value should NOT have changed from 22 (previous test) int rt_val = get_variable_value_int(admin, "mysql-zstd_compression_level", true); - ok(rt_val == 22, "Value 0 rejected, still 22, got: %d", rt_val); + ok(rt_val == zstd_max, "Value 0 rejected, still %d, got: %d", zstd_max, rt_val); } - // Test 6: Reject value 23 (above max) + // Test 6: Reject value above max { - MYSQL_QUERY_T(admin, "SET mysql-zstd_compression_level=23"); + const string q = "SET mysql-zstd_compression_level=" + std::to_string(zstd_max + 1); + int rc = mysql_query(admin, q.c_str()); + ok(rc != 0, "SET mysql-zstd_compression_level=max+1 is rejected"); MYSQL_QUERY_T(admin, "LOAD MYSQL VARIABLES TO RUNTIME"); int rt_val = get_variable_value_int(admin, "mysql-zstd_compression_level", true); - ok(rt_val == 22, "Value 23 rejected, still 22, got: %d", rt_val); + ok(rt_val == zstd_max, "Value above max rejected, still %d, got: %d", zstd_max, rt_val); }Also applies to: 81-96
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/tap/tests/mysql-zstd_compression_level-t.cpp` around lines 73 - 79, The test hardcodes compression bounds (22/23) and uses MYSQL_QUERY_T for operations that should fail; replace magic numbers with the runtime max by calling ZSTD_maxCLevel() to compute valid_max and invalid_max = valid_max + 1, use those when setting and asserting mysql-zstd_compression_level, and for expected-failure statements switch from MYSQL_QUERY_T to the explicit failure-check helper (or assert the query returns an error) so the test asserts failure behavior (refer to mysql-zstd_compression_level, ZSTD_maxCLevel(), MYSQL_QUERY_T, and get_variable_value_int to locate and update the assertions).
130-144:⚠️ Potential issue | 🟠 MajorThe functional check does not prove the zstd path is exercised.
Line 139 only validates that a compressed query works. This can still pass without demonstrating that zstd (and thus
mysql-zstd_compression_level) was the negotiated path.Please assert the negotiated compression algorithm is
zstdbefore counting this case as coverage of the new variable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/tap/tests/mysql-zstd_compression_level-t.cpp` around lines 130 - 144, The test currently only checks that a compressed query succeeds but doesn't verify zstd was actually negotiated; after creating the compressed connection via init_mysql_conn and before closing proxy_cmp, run a follow-up query that returns the negotiated compression algorithm (for example with a session/status variable or a proxy-provided diagnostic query) and assert its value equals "zstd" (use the same mysql_query/mysql_result path you use for the "SELECT 1" check); update the block that uses MYSQL_QUERY_T, init_mysql_conn and proxy_cmp to include this additional assertion so the test proves the zstd path was exercised.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@test/tap/tests/mysql-zstd_compression_level-t.cpp`:
- Around line 73-79: The test hardcodes compression bounds (22/23) and uses
MYSQL_QUERY_T for operations that should fail; replace magic numbers with the
runtime max by calling ZSTD_maxCLevel() to compute valid_max and invalid_max =
valid_max + 1, use those when setting and asserting
mysql-zstd_compression_level, and for expected-failure statements switch from
MYSQL_QUERY_T to the explicit failure-check helper (or assert the query returns
an error) so the test asserts failure behavior (refer to
mysql-zstd_compression_level, ZSTD_maxCLevel(), MYSQL_QUERY_T, and
get_variable_value_int to locate and update the assertions).
- Around line 130-144: The test currently only checks that a compressed query
succeeds but doesn't verify zstd was actually negotiated; after creating the
compressed connection via init_mysql_conn and before closing proxy_cmp, run a
follow-up query that returns the negotiated compression algorithm (for example
with a session/status variable or a proxy-provided diagnostic query) and assert
its value equals "zstd" (use the same mysql_query/mysql_result path you use for
the "SELECT 1" check); update the block that uses MYSQL_QUERY_T, init_mysql_conn
and proxy_cmp to include this additional assertion so the test proves the zstd
path was exercised.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a1f471f7-0f6d-4643-a3aa-a178e302ee3d
📒 Files selected for processing (9)
.gitignoreinclude/MySQL_Thread.hinclude/proxysql_structs.hlib/MySQL_Protocol.cpplib/MySQL_Thread.cpplib/mysql_data_stream.cpptest/tap/groups/groups.jsontest/tap/tests/mysql-zstd_compression_level-t.cpptest/tap/tests/reg_test_2793-compression-t.cpp
✅ Files skipped from review due to trivial changes (1)
- test/tap/groups/groups.json
🚧 Files skipped from review as they are similar to previous changes (2)
- .gitignore
- test/tap/tests/reg_test_2793-compression-t.cpp
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: CI-builds / builds (ubuntu22,-tap)
- GitHub Check: CI-builds / builds (ubuntu24,-tap-genai-gcov)
- GitHub Check: run / trigger
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{cpp,h,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{cpp,h,hpp}: Feature tiers controlled by flags:PROXYSQL31=1for v3.1.x (FFTO, TSDB),PROXYSQLGENAI=1for v4.0.x (GenAI, MCP, Anomaly Detection).PROXYSQLGENAI=1impliesPROXYSQL31=1which impliesPROXYSQLFFTO=1andPROXYSQLTSDB=1
Conditional compilation uses#ifdef PROXYSQLGENAI,#ifdef PROXYSQL31,#ifdef PROXYSQLFFTO,#ifdef PROXYSQLTSDB, and#ifdef PROXYSQLCLICKHOUSEto gate feature-specific code
Class names use PascalCase with protocol prefixes:MySQL_,PgSQL_, orProxySQL_(e.g.,MySQL_Protocol,PgSQL_Session,ProxySQL_Admin)
Member variables use snake_case naming convention
Constants and macros use UPPER_SNAKE_CASE naming convention
Require C++17 standard for all source code
Use RAII (Resource Acquisition Is Initialization) pattern for resource management in C++ code
Use jemalloc for memory allocation in C++ code
Use pthread mutexes for synchronization andstd::atomic<>for counters
Files:
include/proxysql_structs.hlib/MySQL_Protocol.cppinclude/MySQL_Thread.hlib/MySQL_Thread.cpplib/mysql_data_stream.cpptest/tap/tests/mysql-zstd_compression_level-t.cpp
include/**/*.{h,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
Include guards use the pattern
#ifndef __CLASS_*_H(e.g.,#ifndef __ADMIN_H__,#ifndef __PROXYSQL_PROTOCOL_H__)
Files:
include/proxysql_structs.hinclude/MySQL_Thread.h
lib/**/*.cpp
📄 CodeRabbit inference engine (CLAUDE.md)
One class per file is the typical convention in lib/ directory
Files:
lib/MySQL_Protocol.cpplib/MySQL_Thread.cpplib/mysql_data_stream.cpp
test/tap/tests/**/{test_*.cpp,*-t.cpp}
📄 CodeRabbit inference engine (CLAUDE.md)
Test files follow naming pattern
test_*.cppor*-t.cppin test/tap/tests/
Files:
test/tap/tests/mysql-zstd_compression_level-t.cpp
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: sysown/proxysql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-12T11:27:41.156Z
Learning: Applies to **/*.{cpp,h,hpp} : Feature tiers controlled by flags: `PROXYSQL31=1` for v3.1.x (FFTO, TSDB), `PROXYSQLGENAI=1` for v4.0.x (GenAI, MCP, Anomaly Detection). `PROXYSQLGENAI=1` implies `PROXYSQL31=1` which implies `PROXYSQLFFTO=1` and `PROXYSQLTSDB=1`
📚 Learning: 2026-04-12T11:27:41.156Z
Learnt from: CR
Repo: sysown/proxysql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-12T11:27:41.156Z
Learning: Applies to **/*.{cpp,h,hpp} : Feature tiers controlled by flags: `PROXYSQL31=1` for v3.1.x (FFTO, TSDB), `PROXYSQLGENAI=1` for v4.0.x (GenAI, MCP, Anomaly Detection). `PROXYSQLGENAI=1` implies `PROXYSQL31=1` which implies `PROXYSQLFFTO=1` and `PROXYSQLTSDB=1`
Applied to files:
include/proxysql_structs.hlib/MySQL_Protocol.cpplib/MySQL_Thread.cpptest/tap/tests/mysql-zstd_compression_level-t.cpp
📚 Learning: 2026-04-12T11:27:41.156Z
Learnt from: CR
Repo: sysown/proxysql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-12T11:27:41.156Z
Learning: Applies to **/*.{cpp,h,hpp} : Conditional compilation uses `#ifdef PROXYSQLGENAI`, `#ifdef PROXYSQL31`, `#ifdef PROXYSQLFFTO`, `#ifdef PROXYSQLTSDB`, and `#ifdef PROXYSQLCLICKHOUSE` to gate feature-specific code
Applied to files:
include/proxysql_structs.hlib/MySQL_Thread.cpptest/tap/tests/mysql-zstd_compression_level-t.cpp
📚 Learning: 2026-04-12T11:27:41.156Z
Learnt from: CR
Repo: sysown/proxysql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-12T11:27:41.156Z
Learning: Applies to **/*.{cpp,h,hpp} : Class names use PascalCase with protocol prefixes: `MySQL_`, `PgSQL_`, or `ProxySQL_` (e.g., `MySQL_Protocol`, `PgSQL_Session`, `ProxySQL_Admin`)
Applied to files:
lib/MySQL_Protocol.cpp
📚 Learning: 2026-04-01T21:27:03.216Z
Learnt from: wazir-ahmed
Repo: sysown/proxysql PR: 5557
File: test/tap/tests/unit/gtid_set_unit-t.cpp:14-17
Timestamp: 2026-04-01T21:27:03.216Z
Learning: In ProxySQL's unit test directory (test/tap/tests/unit/), test_globals.h and test_init.h are only required for tests that depend on the ProxySQL runtime globals/initialization (i.e., tests that exercise components linked against libproxysql.a). Pure data-structure or utility tests (e.g., ezoption_parser_unit-t.cpp, gtid_set_unit-t.cpp, gtid_trxid_interval_unit-t.cpp) only need tap.h and the relevant project header — omitting test_globals.h and test_init.h is correct and intentional in these cases.
Applied to files:
lib/MySQL_Thread.cpptest/tap/tests/mysql-zstd_compression_level-t.cpp
📚 Learning: 2026-04-12T11:27:41.156Z
Learnt from: CR
Repo: sysown/proxysql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-12T11:27:41.156Z
Learning: Applies to test/tap/tests/unit/**/*.cpp : Unit tests must use `test_globals.h` and `test_init.h` header files and link against `libproxysql.a` via custom test harness
Applied to files:
test/tap/tests/mysql-zstd_compression_level-t.cpp
📚 Learning: 2026-04-12T11:27:41.156Z
Learnt from: CR
Repo: sysown/proxysql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-12T11:27:41.156Z
Learning: Applies to test/tap/tests/**/{test_*.cpp,*-t.cpp} : Test files follow naming pattern `test_*.cpp` or `*-t.cpp` in test/tap/tests/
Applied to files:
test/tap/tests/mysql-zstd_compression_level-t.cpp
📚 Learning: 2026-01-20T09:34:19.124Z
Learnt from: yuji-hatakeyama
Repo: sysown/proxysql PR: 5307
File: test/tap/tests/reg_test_5306-show_warnings_with_comment-t.cpp:39-48
Timestamp: 2026-01-20T09:34:19.124Z
Learning: In ProxySQL's TAP test suite, resource leaks (e.g., not calling mysql_close() on early return paths) are commonly tolerated because test processes are short-lived and OS frees resources on exit. This pattern applies to all C++ test files under test/tap/tests. When reviewing, recognize this as a project-wide test convention and focus on test correctness and isolation rather than insisting on fixing such leaks in these test files.
Applied to files:
test/tap/tests/mysql-zstd_compression_level-t.cpp
📚 Learning: 2026-01-20T07:40:34.938Z
Learnt from: yuji-hatakeyama
Repo: sysown/proxysql PR: 5307
File: test/tap/tests/reg_test_5306-show_warnings_with_comment-t.cpp:24-28
Timestamp: 2026-01-20T07:40:34.938Z
Learning: In ProxySQL test files, calling `mysql_error(NULL)` after `mysql_init()` failure is safe because the MariaDB client library implementation returns an empty string for NULL handles (not undefined behavior).
Applied to files:
test/tap/tests/mysql-zstd_compression_level-t.cpp
📚 Learning: 2026-03-26T16:39:02.446Z
Learnt from: yuji-hatakeyama
Repo: sysown/proxysql PR: 5548
File: lib/mysql_connection.cpp:1837-1843
Timestamp: 2026-03-26T16:39:02.446Z
Learning: In ProxySQL's lib/mysql_connection.cpp, `SHOW WARNINGS` detection for both `update_warning_count_from_connection()` and the `add_eof()` call in `ASYNC_USE_RESULT_CONT` intentionally uses `myds->sess->CurrentQuery.QueryParserArgs.digest_text` (comment-stripped digest text). This means the fix/feature does not work when `mysql-query_digests_keep_comment=1` (digest_text contains comments) or `mysql-query_digests=0` (digest_text is unavailable) — these configurations are explicitly excluded from the regression test for `reg_test_5306-show_warnings_with_comment-t`. This design is consistent across the codebase and is an accepted, documented limitation.
Applied to files:
test/tap/tests/mysql-zstd_compression_level-t.cpp
📚 Learning: 2026-02-13T09:29:39.713Z
Learnt from: wazir-ahmed
Repo: sysown/proxysql PR: 5372
File: test/tap/tap/mcp_client.cpp:355-385
Timestamp: 2026-02-13T09:29:39.713Z
Learning: In ProxySQL MCP implementation (test/tap/tap/mcp_client.cpp), the `check_server()` method uses the ping endpoint which is designed to work without authentication. The `ping` method at the `config` endpoint should not require the `Authorization: Bearer` header, unlike tool invocation endpoints which do require authentication when `auth_token_` is set.
Applied to files:
test/tap/tests/mysql-zstd_compression_level-t.cpp
🪛 Clang (14.0.6)
test/tap/tests/mysql-zstd_compression_level-t.cpp
[error] 12-12: 'cstdio' file not found
(clang-diagnostic-error)
🪛 Cppcheck (2.20.0)
lib/mysql_data_stream.cpp
[warning] 32-32: If memory allocation fails, then there is a possible null pointer dereference
(nullPointerOutOfMemory)
🪛 GitHub Check: SonarCloud Code Analysis
lib/MySQL_Thread.cpp
[failure] 532-532: C-style cast removing const qualification from the type of a pointer may lead to an undefined behaviour.
🔇 Additional comments (5)
include/MySQL_Thread.h (1)
491-491: Looks good: dedicated zstd config field is correctly introduced.Line 491 cleanly adds the independent
zstd_compression_levelknob in the thread handler variable set.lib/mysql_data_stream.cpp (1)
32-33: Correct fallback source switch for zstd compression level.Lines 32-33 correctly decouple zstd fallback from
mysql_thread___protocol_compression_levelwhile preserving range validation.lib/MySQL_Protocol.cpp (1)
2303-2303: Fallback clamp now uses the right thread-local zstd variable.Line 2303 correctly clamps
mysql_thread___zstd_compression_leveland keeps safe bounds for negotiated zstd usage.include/proxysql_structs.h (1)
1299-1299: Thread-local declaration wiring is complete and consistent.Lines 1299 and 1633 correctly add both the TLS definition and extern declaration for
mysql_thread___zstd_compression_level.Also applies to: 1633-1633
lib/MySQL_Thread.cpp (1)
33-33: Complete and consistent zstd variable wiring.The new
zstd_compression_levelpath is correctly registered, defaulted, range-validated, and refreshed at thread scope.Also applies to: 532-532, 1458-1458, 2675-2675, 4740-4740, 4825-4825
1 participant
Summary
Closes #5632
ProxySQL v3.0.7 introduced zstd compression but shared
mysql-protocol_compression_levelwith zlib. This is incorrect because the two algorithms have fundamentally different level semantics:ZSTD_maxCLevel())The shared variable range
[-1, 9]made zstd levels 10–22 inaccessible and the same numeric value meant very different compression ratios for each algorithm.This PR adds a dedicated
mysql-zstd_compression_levelvariable (range 1–22, default 3) so that zstd and zlib compression can be configured independently.Changes
Core variable infrastructure
include/proxysql_structs.h: Thread-local__thread int mysql_thread___zstd_compression_leveldeclaration and extern.include/MySQL_Thread.h:int zstd_compression_levelstruct member in MySQL_Thread variables.lib/MySQL_Thread.cpp: Full variable registration — name array, constructor init,VariablesPointers_intwith range[1, ZSTD_maxCLevel()], refresh macro, thread-local default.Wire into compression logic
lib/MySQL_Protocol.cpp: Handshake zstd level selection now usesmysql_thread___zstd_compression_levelinstead ofmysql_thread___protocol_compression_level.lib/mysql_data_stream.cpp:get_zstd_compression_level()fallback path now uses the dedicated variable.Tests
test/tap/tests/mysql-zstd_compression_level-t.cpp(new): 10 test cases covering:test/tap/tests/reg_test_2793-compression-t.cpp: Added variable visibility check via admin interface.test/tap/groups/groups.json: Registered new test in same groups asmysql-protocol_compression_level-t.Housekeeping
.gitignore: Addedtest/tap/tests/unit/*-tpattern for orphan unit test binaries.Backwards Compatibility
mysql-protocol_compression_levelis unchanged — continues to control zlib compression only.Test plan
make clean && make debug -j32)make build_tap_test_debug)mysql-zstd_compression_level-tpasses all 10 tests against mysql84-g1groups.jsonvalid and consistent (check_groups.py)Summary by CodeRabbit
New Features
mysql-zstd_compression_levelvariable (default: 3, range: 1–max).Tests