PBS-3: Introduce new <replication_source> section into the main configuration file - #186
Merged
Merged
Conversation
kamil-holubicki
force-pushed
the
PBS-3
branch
2 times, most recently
from
September 21, 2026 19:46
17f23bf to
b9d2197
Compare
…stener Introduce a new top-level "replication_source" JSON configuration section that controls the MySQL-compatible listener the utility exposes in 'pull' mode. The section carries three fields: - port : TCP port the listener binds to - read_timeout : per-frame read timeout applied to accepted sessions - write_timeout : per-frame write timeout applied to accepted sessions replication_source is a required section validated at config load time (port, read_timeout and write_timeout must all be non-zero). The README and the sample main_config.json are updated accordingly. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Now that the Binlog Server pull-mode listener reads its TCP port from the replication_source config section instead of hard-coding 3307, generate_binsrv_config.inc emits a replication_source block whose port is @@global.port + 1000. Because @@global.port is unique per MTR worker, so is the derived listener port, so pull-mode tests no longer collide when run in parallel. The +1000 offset intentionally jumps well beyond MTR's per-worker port allocation (empirically MTR reserves ports within a few hundred of each worker's @@PORT on GitHub Actions runners), so the derived listener port never lands inside a sibling worker's reserved range. The computed value is exported to callers via $binsrv_replication_source_port for tests that need to talk to the listener themselves. Consequently: - pull_mode.test, binlog_flush.test and auth_method_switch.test drop their '--source include/not_parallel.inc' guards. - auth_method_switch.test's /dev/tcp probe and the two mysql CLI invocations that connect to the listener now use $binsrv_replication_source_port instead of hard-coded 3307; the matching .result echo is rewritten in port-agnostic form. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
kamil-holubicki
force-pushed
the
PBS-3
branch
from
September 21, 2026 21:42
e4cf776 to
9e17862
Compare
pull_mode intentionally runs binsrv with $binsrv_read_timeout=3s and $binsrv_idle_time=1s so the source-mysqld restarts in the test force binsrv through several reconnect cycles. Each cycle is ~5s wall time and the test walks through three binlog files before checking that the fourth one appears. On release/debug builds the 60-iteration (60s) grep loop finishes with plenty of headroom, but under sanitized (ASAN) builds and parallel MTR execution each cycle stretches and 60s runs out before the fourth binlog shows up in the log. Bump $max_number_of_attempts from 60 to 300 (5 minutes). The loop exits on the first hit, so healthy runs pay nothing; it is still an order of magnitude smaller than the enclosing --testcase-timeout so a genuinely stuck binsrv still fails inside its intended envelope. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
kamil-holubicki
force-pushed
the
PBS-3
branch
from
September 21, 2026 21:44
9e17862 to
468826f
Compare
percona-ysorokin
approved these changes
Sep 21, 2026
percona-ysorokin
left a comment
percona-ysorokin
left a comment
Collaborator
There was a problem hiding this comment.
LGTM with 1 suggested improvement
| network_service(binsrv::basic_logger_ptr logger, | ||
| boost::asio::io_context &context, binsrv::storage_ptr storage, | ||
| std::uint16_t listening_port, std::string_view username, | ||
| std::uint16_t listening_port, |
Collaborator
There was a problem hiding this comment.
May be make network_service constructor accept const replication_source_config &cfg instead of passing individual components.?
Contributor
Author
There was a problem hiding this comment.
Let's do this in next PR, where user credentials will come from config as well.
One downside of sucha a decision: we introduce a new dependency of network_service class. Now it needs to know about config class and about schema it implements.
Advantage: cleaner api.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The documentation incorrectly claims that the listener streams collected binary logs despite its sample-only implementation.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
What changed in this PR
Adds configurable replication-source listener ports and I/O timeouts to the main configuration.
Changes:
- Introduces and validates the
replication_sourceconfiguration section. - Passes configured port and timeouts into the MySQL-compatible listener.
- Enables parallel MTR execution using worker-specific listener ports.
| File | Description |
|---|---|
CMakeLists.txt |
Registers new configuration sources. |
main_config.json |
Adds example listener settings. |
README.md |
Documents the new section. |
src/binsrv/main_config.cpp |
Validates listener configuration. |
src/binsrv/main_config.hpp |
Adds the section to the schema. |
src/binsrv/replication_source_config.cpp |
Validates port and timeouts. |
src/binsrv/replication_source_config.hpp |
Defines listener configuration. |
src/binsrv/replication_source_config_fwd.hpp |
Adds its forward declaration. |
src/minimysql/network_service.cpp |
Applies configurable timeouts. |
src/minimysql/network_service.hpp |
Extends the service constructor. |
src/operations/pull_operation.cpp |
Passes configuration to the listener. |
mtr/binlog_streaming/include/generate_binsrv_config.inc |
Generates worker-specific settings. |
mtr/binlog_streaming/t/auth_method_switch.test |
Uses the configured listener port. |
mtr/binlog_streaming/r/auth_method_switch.result |
Updates expected output. |
mtr/binlog_streaming/t/binlog_flush.test |
Enables parallel execution. |
mtr/binlog_streaming/t/pull_mode.test |
Enables parallel execution and extends waiting. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| - `<replication.rewrite.file_size>` - the maximum individual binlog file size after reaching which the utility will switch to a new one. The value is expected to be a string containing an integer followed by an optional suffix 'K' / 'M' / 'G' / 'T' / 'P', e.g. /\d+\[KMGTP\]?/. The minimal allowed value of this parameter is `1024` bytes. | ||
|
|
||
| #### \<replication_source\> section | ||
| This section configures the built-in MySQL-compatible listener the utility exposes in `pull` mode so that downstream replicas can dump binary log events from it (the utility acts as a replication source). |
kamil-holubicki
force-pushed
the
PBS-3
branch
2 times, most recently
from
September 22, 2026 07:36
6759894 to
468826f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.