Repository navigation
More changes to k8s manifest files. - #58
Merged
Merged
Conversation
Collaborator
minguyen9988
added a commit
that referenced
this pull request
Aug 25, 2026
) * fix: an excluded keyless table must not block startup (#58) The keyless-table preflight refused to start whenever the SOURCE held a table with no row identity, even when the operator had already excluded that table from replication. A table the connector never reads cannot be replicated incorrectly, so it has no business blocking startup. The practical effect was that the only ways past a keyless table were to add a primary key to a table you may not own, or to set keyless.table.check.skip=true and lose the protection for every OTHER table on the source. Neither is right when the table is already excluded. Concretely: txnrepo uat and staging carry alembic_version in 18 databases (Alembic schema bookkeeping, 0-1 rows, no key). Excluding it is the correct fix, but before this change the connector would still refuse to start. withoutExcluded() drops out-of-scope tables before the refusal decision, honouring both Debezium list properties with Debezium semantics: * entries are REGULAR EXPRESSIONS, full-matched against the fully-qualified db.table -- not substring, so an exclusion written for one table cannot silently swallow a neighbour; * table.include.list, when set, means anything NOT listed is out of scope. Fails safe in the same direction as databaseFilter(): an uncompilable pattern is IGNORED rather than treated as a match. This list can only ever REMOVE tables from the refusal set, so a broken pattern that matched everything would silently drop a genuinely keyless table from the check - a false pass, which is worse than a false refusal. Tests: KeylessTablePreflightTest 20 (5 new) - an excluded keyless table no longer blocks (using the exact ".*.temp_.*,.*[.]alembic_version" pattern shipped for txnrepo uat/staging, so the test fails if that production exclusion ever stops covering the table it was written for); exclusion is anchored not substring; a table outside table.include.list is out of scope; no lists configured excludes nothing; a malformed pattern is ignored while the valid entry beside it still applies. Failing-first verified: with withoutExcluded() short-circuited, 3 of the 5 fail, each showing the excluded table still blocking. Full suite green: 44 across KeylessTablePreflightTest, SequenceSeedOverflowTest, SourceTsVersionAnchorTest, DdlReplayIdempotencyTest, CreateTableNoKeySortKeyTest. Co-authored-by: Jenkins Continuous Build server <jenkins@fpif1-dbal2.w2k.jumptrading.com> * fix: the keyless preflight must not reconfigure the source server CI run 32819328349 failed java-tests-lightweight with 3 failures and 19 errors across 15 suites. Two causes, both from the keyless preflight. 1. THE PREFLIGHT WROTE A GLOBAL ON THE SOURCE (a real defect, not a test problem). enableGipk() ran SET GLOBAL sql_generate_invisible_primary_key = ON With that global on, MySQL REJECTS every subsequent keyless CREATE TABLE ... PARTITION BY on the WHOLE server: ERROR 1235 (42000): This version of MySQL doesn't yet support 'generating invisible primary key for the partitioned tables' So merely starting the connector broke partitioned-table DDL for every application sharing that MySQL -- an outage on the SOURCE caused by a read-only consumer, persisting after the connector stopped, and lasting until someone found the global and reset it. TableOperationsIT:111 is exactly that: a CREATE TABLE ... PARTITION BY KEY(joined) rejected on a server the connector had reconfigured. The global is now never written. The session setting is kept (harmless; the connector creates no source tables). When the global is OFF the passing path logs a WARN with the exact statements and states the partitioned-table consequence, so enabling it stays the server owner's decision, made with knowledge of what else uses that MySQL. The refusal message already carried the same advice. 2. THE SHARED IT FIXTURE IS KEYLESS ON PURPOSE. sql/data_types.sql declares ship_class and add_test with no PRIMARY KEY and no UNIQUE key because the DDL-parser suites exercise ALTER/RENAME/TRUNCATE against them. The preflight is right to refuse such a source, so the ITs opt out via keyless.table.check.skip in src/test/resources/config.yml, with a comment explaining why. The check itself stays covered directly by KeylessTablePreflightTest, which does not depend on that fixture. The remaining 17 errors in the run are downstream of these two: once the connector refuses to start, the ITs' ClickHouse tables are never created and the assertions fail with UNKNOWN_TABLE / UNKNOWN_IDENTIFIER, and one suite reports "Mapped port can only be obtained after the container is started" after its container teardown. Tests: KeylessTablePreflightTest 22 (2 new). - testPreflightNeverSetsAGlobalOnTheSource asserts against the source text, because the failure mode is the PRESENCE of the statement: any execution path reaching it is the bug, so no runtime scenario has to be guessed at. - testItConfigOptsOutOfTheKeylessCheck pins the opt-out so a future edit cannot silently reintroduce a 19-error run. Both locate their file from the compiled class location and fail loudly if it is missing, so neither can pass vacuously. Failing-first verified. Restoring the SET GLOBAL call and removing the opt-out fails exactly the two new tests: testPreflightNeverSetsAGlobalOnTheSource:370 the preflight must not execute SET GLOBAL on the source server: st.execute("SET GLOBAL sql_generate_invisible_primary_key = ON"); testItConfigOptsOutOfTheKeylessCheck:401 keyless.table.check.skip must be true in .../config.yml Full unit run green: 154 across KeylessTablePreflightTest, SequenceSeedOverflowTest, SourceTsVersionAnchorTest, DdlReplayIdempotencyTest, CreateTableNoKeySortKeyTest, MySqlDDLParserListenerImplTest. * chore: drop build-generated churn in dependency-reduced-pom.xml Inherited from the base branch, where a test run let maven-shade regenerate the file: all 441 lines rewritten with CRLF endings plus a silent bump of the checked-in lombok pin from 1.18.22 to 1.18.34. Neither belongs in this change, and against upstream 2.10.0 it showed up as a 441-line third file in the diff. Restored to the 2.10.0 content. Verified before reverting that after CRLF normalisation the lombok <version> line was the only content difference, so nothing else is lost. * fix: read the global once, and record why the pom is left untouched Review findings from round 2. 1. gipkEnabledGlobally() ran the same SELECT twice whenever the first comparison did not match -- i.e. on every server where the global is OFF, exactly the path that exists in order to warn. Now one round trip into a local, with both renderings (ON/OFF and 1/0) still accepted. 2. All three judges flagged a lombok skew in dependency-reduced-pom.xml, and they were right to look: sink-connector-lightweight/pom.xml declares 1.18.34 in BOTH the dependency and the compiler annotationProcessorPaths, while the checked-in generated dependency-reduced-pom.xml carries 1.18.34 in the processor path and 1.18.22 in the dependency block. The committed generated file is the stale, internally inconsistent one. No change is made here. That file is build-generated, its inconsistency predates this work, and it is already byte-identical to the base commit on this branch, so it does not appear in the diff. Regenerating or gitignoring it is a separate call for the repo's owners; this PR should not carry an unrelated dependency change either direction. Unit run green: 154 across KeylessTablePreflightTest, SequenceSeedOverflowTest, SourceTsVersionAnchorTest, DdlReplayIdempotencyTest, CreateTableNoKeySortKeyTest, MySqlDDLParserListenerImplTest. --------- Co-authored-by: svc_dba_github <dbajobs@jumptrading.com> Co-authored-by: Jenkins Continuous Build server <jenkins@fpif1-dbal2.w2k.jumptrading.com>
minguyen9988
added a commit
that referenced
this pull request
Aug 25, 2026
* fix: an excluded keyless table must not block startup (#58) The keyless-table preflight refused to start whenever the SOURCE held a table with no row identity, even when the operator had already excluded that table from replication. A table the connector never reads cannot be replicated incorrectly, so it has no business blocking startup. The practical effect was that the only ways past a keyless table were to add a primary key to a table you may not own, or to set keyless.table.check.skip=true and lose the protection for every OTHER table on the source. Neither is right when the table is already excluded. Concretely: txnrepo uat and staging carry alembic_version in 18 databases (Alembic schema bookkeeping, 0-1 rows, no key). Excluding it is the correct fix, but before this change the connector would still refuse to start. withoutExcluded() drops out-of-scope tables before the refusal decision, honouring both Debezium list properties with Debezium semantics: * entries are REGULAR EXPRESSIONS, full-matched against the fully-qualified db.table -- not substring, so an exclusion written for one table cannot silently swallow a neighbour; * table.include.list, when set, means anything NOT listed is out of scope. Fails safe in the same direction as databaseFilter(): an uncompilable pattern is IGNORED rather than treated as a match. This list can only ever REMOVE tables from the refusal set, so a broken pattern that matched everything would silently drop a genuinely keyless table from the check - a false pass, which is worse than a false refusal. Tests: KeylessTablePreflightTest 20 (5 new) - an excluded keyless table no longer blocks (using the exact ".*.temp_.*,.*[.]alembic_version" pattern shipped for txnrepo uat/staging, so the test fails if that production exclusion ever stops covering the table it was written for); exclusion is anchored not substring; a table outside table.include.list is out of scope; no lists configured excludes nothing; a malformed pattern is ignored while the valid entry beside it still applies. Failing-first verified: with withoutExcluded() short-circuited, 3 of the 5 fail, each showing the excluded table still blocking. Full suite green: 44 across KeylessTablePreflightTest, SequenceSeedOverflowTest, SourceTsVersionAnchorTest, DdlReplayIdempotencyTest, CreateTableNoKeySortKeyTest. Co-authored-by: Jenkins Continuous Build server <jenkins@fpif1-dbal2.w2k.jumptrading.com> * fix: the keyless preflight must not reconfigure the source server CI run 32819328349 failed java-tests-lightweight with 3 failures and 19 errors across 15 suites. Two causes, both from the keyless preflight. 1. THE PREFLIGHT WROTE A GLOBAL ON THE SOURCE (a real defect, not a test problem). enableGipk() ran SET GLOBAL sql_generate_invisible_primary_key = ON With that global on, MySQL REJECTS every subsequent keyless CREATE TABLE ... PARTITION BY on the WHOLE server: ERROR 1235 (42000): This version of MySQL doesn't yet support 'generating invisible primary key for the partitioned tables' So merely starting the connector broke partitioned-table DDL for every application sharing that MySQL -- an outage on the SOURCE caused by a read-only consumer, persisting after the connector stopped, and lasting until someone found the global and reset it. TableOperationsIT:111 is exactly that: a CREATE TABLE ... PARTITION BY KEY(joined) rejected on a server the connector had reconfigured. The global is now never written. The session setting is kept (harmless; the connector creates no source tables). When the global is OFF the passing path logs a WARN with the exact statements and states the partitioned-table consequence, so enabling it stays the server owner's decision, made with knowledge of what else uses that MySQL. The refusal message already carried the same advice. 2. THE SHARED IT FIXTURE IS KEYLESS ON PURPOSE. sql/data_types.sql declares ship_class and add_test with no PRIMARY KEY and no UNIQUE key because the DDL-parser suites exercise ALTER/RENAME/TRUNCATE against them. The preflight is right to refuse such a source, so the ITs opt out via keyless.table.check.skip in src/test/resources/config.yml, with a comment explaining why. The check itself stays covered directly by KeylessTablePreflightTest, which does not depend on that fixture. The remaining 17 errors in the run are downstream of these two: once the connector refuses to start, the ITs' ClickHouse tables are never created and the assertions fail with UNKNOWN_TABLE / UNKNOWN_IDENTIFIER, and one suite reports "Mapped port can only be obtained after the container is started" after its container teardown. Tests: KeylessTablePreflightTest 22 (2 new). - testPreflightNeverSetsAGlobalOnTheSource asserts against the source text, because the failure mode is the PRESENCE of the statement: any execution path reaching it is the bug, so no runtime scenario has to be guessed at. - testItConfigOptsOutOfTheKeylessCheck pins the opt-out so a future edit cannot silently reintroduce a 19-error run. Both locate their file from the compiled class location and fail loudly if it is missing, so neither can pass vacuously. Failing-first verified. Restoring the SET GLOBAL call and removing the opt-out fails exactly the two new tests: testPreflightNeverSetsAGlobalOnTheSource:370 the preflight must not execute SET GLOBAL on the source server: st.execute("SET GLOBAL sql_generate_invisible_primary_key = ON"); testItConfigOptsOutOfTheKeylessCheck:401 keyless.table.check.skip must be true in .../config.yml Full unit run green: 154 across KeylessTablePreflightTest, SequenceSeedOverflowTest, SourceTsVersionAnchorTest, DdlReplayIdempotencyTest, CreateTableNoKeySortKeyTest, MySqlDDLParserListenerImplTest. * chore: drop build-generated churn in dependency-reduced-pom.xml Inherited from the base branch, where a test run let maven-shade regenerate the file: all 441 lines rewritten with CRLF endings plus a silent bump of the checked-in lombok pin from 1.18.22 to 1.18.34. Neither belongs in this change, and against upstream 2.10.0 it showed up as a 441-line third file in the diff. Restored to the 2.10.0 content. Verified before reverting that after CRLF normalisation the lombok <version> line was the only content difference, so nothing else is lost. * fix: read the global once, and record why the pom is left untouched Review findings from round 2. 1. gipkEnabledGlobally() ran the same SELECT twice whenever the first comparison did not match -- i.e. on every server where the global is OFF, exactly the path that exists in order to warn. Now one round trip into a local, with both renderings (ON/OFF and 1/0) still accepted. 2. All three judges flagged a lombok skew in dependency-reduced-pom.xml, and they were right to look: sink-connector-lightweight/pom.xml declares 1.18.34 in BOTH the dependency and the compiler annotationProcessorPaths, while the checked-in generated dependency-reduced-pom.xml carries 1.18.34 in the processor path and 1.18.22 in the dependency block. The committed generated file is the stale, internally inconsistent one. No change is made here. That file is build-generated, its inconsistency predates this work, and it is already byte-identical to the base commit on this branch, so it does not appear in the diff. Regenerating or gitignoring it is a separate call for the repo's owners; this PR should not carry an unrelated dependency change either direction. Unit run green: 154 across KeylessTablePreflightTest, SequenceSeedOverflowTest, SourceTsVersionAnchorTest, DdlReplayIdempotencyTest, CreateTableNoKeySortKeyTest, MySqlDDLParserListenerImplTest. * harden: the connector must be read-only against its MySQL source A replication consumer must never change the server it reads from. #59 fixed one violation of that rule; this makes the rule structural, so the next one cannot be introduced silently. SCOPE. Swept both modules for statements executed against the SOURCE. KeylessTablePreflight is the connector's ONLY code path that opens its own connection to MySQL -- everything else either targets ClickHouse (the sink, where writing is the job) or is Debezium reading the binlog. So the guarantee is enforced at that one chokepoint. WHAT WAS STILL THERE. One write remained: SET SESSION sql_generate_invisible_primary_key = ON. It is harmless in effect -- the setting only applies to CREATE TABLE, and the preflight only reads information_schema, so it changed nothing on any server. But it is still a write issued by a read-only program, and leaving it meant the class could not claim, or enforce, read-only. Removed. Nothing depended on it: it was already dead weight after #59 dropped the global. THREE LAYERS, so the guarantee does not rest on reviewers noticing: 1. Connection.setReadOnly(true) on the preflight's connection. The MySQL driver sends SET SESSION TRANSACTION READ ONLY, so the SERVER rejects a write even if future code asks for one. This is the only layer that does not depend on the connector behaving. Requested and then VERIFIED via isReadOnly(), because a driver that silently ignored it would leave the guarantee resting on the other two without saying so. Note this is a session property of a short-lived connection owned by the connector -- it changes nothing for any other client, which is precisely the distinction the old SET GLOBAL got wrong. 2. assertReadOnlySql() on every statement, refusing a write before it is sent, with a message naming the rule instead of a driver error. Deliberately an ALLOWLIST (SELECT only): a denylist is the wrong shape for a safety check, because every statement a future MySQL adds defaults to permitted -- and the verb that actually caused the incident, SET, is easy to omit precisely because it does not read like a write. Leading /* */, -- and # comments are stripped first, so a commented statement cannot slip past the prefix test. 3. testPreflightIssuesNoWritesToTheSource fails the BUILD if any of 25 mutating verbs appears in an executed string literal in this class. This is the general form of #59's SET GLOBAL test: that one pins a statement, this pins the principle, so the next write fails even though nobody predicted its verb. The operator-advice message still quotes SET GLOBAL, because telling the operator how to enable GIPK themselves is the whole point of that message. It is log text, never executed, and the scan skips only that shape (a concatenated message fragment ending in ";\n") -- an actual write would not look like it. Behaviour is otherwise unchanged: the check still refuses a source with pre-existing keyless tables, still refuses below 8.0.30, and still reports whether GIPK is ON with the statements to enable it. Tests: KeylessTablePreflightTest 27 (5 new) -- the no-writes scan, writes rejected, the reads this class issues allowed, comment-disguised writes rejected, and non-SELECT statements refused rather than assumed safe. Failing-first verified with a verb the guard's call sites never mention: adding probe.execute("FLUSH TABLES WITH READ LOCK") fails the build with the connector is read-only against its MySQL source, but this class contains a 'FLUSH' statement: probe.execute("FLUSH TABLES WITH READ LOCK"); Unit run green: 159 across KeylessTablePreflightTest, SequenceSeedOverflowTest, SourceTsVersionAnchorTest, DdlReplayIdempotencyTest, CreateTableNoKeySortKeyTest, MySqlDDLParserListenerImplTest. Destructive-Op-Check: sites=2; result=pass * harden: the connector must be read-only against its MySQL source A replication consumer must never change the server it reads from. #1406 fixed one violation of that rule; this makes the rule structural, so the next one cannot be introduced silently. SCOPE. Swept both modules for statements executed against the SOURCE. KeylessTablePreflight is the connector's ONLY code path that opens its own connection to MySQL -- everything else either targets ClickHouse (the sink, where writing is the job) or is Debezium reading the binlog. So the guarantee is enforced at that one chokepoint. WHAT WAS STILL THERE. One write remained: SET SESSION sql_generate_invisible_primary_key = ON. It is harmless in effect -- the setting only applies to CREATE TABLE, and the preflight only reads information_schema, so it changed nothing on any server. But it is still a write issued by a read-only program, and leaving it meant the class could not claim, or enforce, read-only. Removed. Nothing depended on it: it was already dead weight after #1406 dropped the global. THREE LAYERS, so the guarantee does not rest on reviewers noticing: 1. Connection.setReadOnly(true) on the preflight's connection. The MySQL driver sends SET SESSION TRANSACTION READ ONLY, so the SERVER rejects a write even if future code asks for one. This is the only layer that does not depend on the connector behaving. Requested and then VERIFIED via isReadOnly(), because a driver that silently ignored it would leave the guarantee resting on the other two without saying so. Note this is a session property of a short-lived connection owned by the connector -- it changes nothing for any other client, which is precisely the distinction the old SET GLOBAL got wrong. 2. assertReadOnlySql() on every statement, refusing a write before it is sent, with a message naming the rule instead of a driver error. Deliberately an ALLOWLIST (SELECT only): a denylist is the wrong shape for a safety check, because every statement a future MySQL adds defaults to permitted -- and the verb that actually caused the incident, SET, is easy to omit precisely because it does not read like a write. Leading /* */, -- and # comments are stripped first, so a commented statement cannot slip past the prefix test. 3. testPreflightIssuesNoWritesToTheSource fails the BUILD if any of 25 mutating verbs appears in an executed string literal in this class. This is the general form of #59's SET GLOBAL test: that one pins a statement, this pins the principle, so the next write fails even though nobody predicted its verb. The operator-advice message still quotes SET GLOBAL, because telling the operator how to enable GIPK themselves is the whole point of that message. It is log text, never executed, and the scan skips only that shape (a concatenated message fragment ending in ";\n") -- an actual write would not look like it. Behaviour is otherwise unchanged: the check still refuses a source with pre-existing keyless tables, still refuses below 8.0.30, and still reports whether GIPK is ON with the statements to enable it. Tests: KeylessTablePreflightTest 27 (5 new) -- the no-writes scan, writes rejected, the reads this class issues allowed, comment-disguised writes rejected, and non-SELECT statements refused rather than assumed safe. Failing-first verified with a verb the guard's call sites never mention: adding probe.execute("FLUSH TABLES WITH READ LOCK") fails the build with the connector is read-only against its MySQL source, but this class contains a 'FLUSH' statement: probe.execute("FLUSH TABLES WITH READ LOCK"); Unit run green: 159 across KeylessTablePreflightTest, SequenceSeedOverflowTest, SourceTsVersionAnchorTest, DdlReplayIdempotencyTest, CreateTableNoKeySortKeyTest, MySqlDDLParserListenerImplTest. Destructive-Op-Check: sites=2; result=pass --------- Co-authored-by: svc_dba_github <dbajobs@jumptrading.com> Co-authored-by: Jenkins Continuous Build server <jenkins@fpif1-dbal2.w2k.jumptrading.com> Co-authored-by: svc_dba_github <svc_dba_github@fpif1-vcsl1.w2k.jumptrading.com>
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.
Newer image and fixes for strimzi image.