Repository navigation
test: add Kafka Connect recovery qualification harness - #30025
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
mo-bot review · Commit: Review completed. Read the assessment. |
XuPeng-SH
left a comment
XuPeng-SH
left a comment
There was a problem hiding this comment.
mo-bot review · mo-bot-local · review
Commit: fa339bbb0f727be7aae6a474d55bbb0c5cfad98f
Assessment: request_changes
Reviewed all eight changed files and affected callers. The recovery oracle and shared lifecycle ownership are reasonable, but two reproducible reporting/CLI defects remain.
Model: gpt-6-astra / xhigh
Session: 01a12d8f-8e48-7622-ad17-b605f08450ce · slot 1 · generation 0
-
P2
optools/kafka_connect_ci.bash:168: Reuse MO_KAFKA_CONNECT_REPORT_DIR after a successful run, then fail during setup or driver compilation before a new report is written, with cleanup otherwise succeeding.
Impact: The runner exits nonzero but retains the previous report.json marked passed and emits no setup-failure.json. Initialize or isolate each run's report before setup and record its failure regardless of an existing report.
Evidence: Executed the checked-in cleanup body with external dependencies stubbed successful: shell exit=1, prior passed report unchanged, setup-failure.json absent. Retained stale-report-check.log and checks/1791955523122215103-2.json. -
P2
test/kafkaconnect/kafka_connect_e2e_local.go:1122: Run the Go driver from the repository root with an owned --compose-project and the default relative Compose paths.
Impact: Changing Docker's working directory while retaining repository-relative -f arguments produces duplicated paths. Worker control fails, preventing recovery qualification. Resolve both paths before changing directory or retain the original working directory.
Evidence: Defaults at lines 259–260 resolve under cmd.Dir to etc/launch-kafka-local/etc/launch-kafka-local/compose*.yaml, which do not exist. Confirmed in relative-compose-paths.log and checks/1791955597076003674-2.json. The shell wrapper and existing helper tests use absolute paths.
Validation:
-
Completed recovery ordering, exact-row/offset oracle, cancellation, cleanup, MongoDB compatibility, test-quality and documentation review.
-
Matched focused checks to clean-head command/source/exit receipts. Static audit passed patch/blob reconciliation, formatting, shell syntax and generated TOML/port layout: checks/1791955654684992514-2.json and source-evidence-audit.log. All artifacts are under the assigned attempt-1 directory.
-
Frozen current-head CI records successful Ubuntu UT, arm64 SCA, coverage, PROXY/PESSIMISTIC BVT and CI Required. These do not establish real Kafka Connect E2E execution.
Limitations:
-
No Go suites or native services were run locally. local_validation is empty; author counts lack supplied raw receipts. The reported runtime smoke used an older MatrixOne binary and preceded final hardening.
-
The linked issue body was unavailable. Review followed the supplied bounded qualification contract; broader connector qualification remains outside its claims.
-
No product SQL hot path changes. Fixture work uses seven records and four phases; no performance benchmark was supplied or inferred.
aptend
left a comment
aptend
left a comment
There was a problem hiding this comment.
Deep re-review: APPROVE at exact head 2922b56, base/merge-base 1191452.
Reviewed all eight changed files and the fa339bb->2922b56f increment; read the complete formal review/conversation history (no inline threads) and the linked qualification contract. This is a bounded test/CI qualification update and existing-helper extraction, not a new MatrixOne product/API contract; no feature-design gate applies.
Both prior findings are independently closed. In a separate exact-fa339bbb worktree, the two new regression oracles fail: preflight preserves a stale passed report, and default relative Compose paths remain relative after cmd.Dir changes. Both tests pass at 2922b56, without production overlays.
Reviewed target-row/NULL/empty/duplicate-ID oracles, all three independently obtained offsets, stop->STOPPED->PATCH->resume ordering, same-worker crash recovery without re-registration/reset, replay and empty restart, report replacement/error propagation, HTTP/SQL/Kafka cancellation and cleanup, pinned Compose configuration, and shared MongoDB port/config/lifecycle consumers. The seven-record fixture is proportionate; idempotent final state is not claimed as exactly-once business delivery.
Independent Linux/amd64 / Go1.27.1 evidence:
- Kafka Connect package: 30 top-level / 46 named tests pass; owning race suite passes with zero skips/failures.
- MongoDB consumer package: all 14 top-level tests pass normally and under race.
- Individually measured/adaptively repeated race tests pass in one process per selection: recovery orchestration T=1.01s,N=29; dependency failures T=.03s,N=100; in-process Kafka T=.03s,N=100; cross-process lease T=.13s,N=100; parent-death lease release T=.12s,N=100 (B=30s, cap100).
- Native artifacts built by make cgo; the checked-in e2e-local wrapper then built actual MatrixOne executables from this head. Two complete real Kafka3.9.0 / Connect3.9.0 / Debezium JDBC3.1.2.Final runs passed, reusing the report directory. Each run preserves one MatrixOne/broker/Connect fixture across the four recovery phases. Baseline REST/group/log-end offsets are all3; catch-up/replay/empty-restart offsets are all7. Exact final IDs2,3,4,5, updated Unicode/counter33, empty note versus SQL NULL, and deletion of ID1 are verified. Both runner exits are0; independent post-run checks find no owned containers/volumes/networks/service process and can reacquire the port lease.
- Executable SHA256s: 5217716301dc1ffca1b800703aff7afa3222ba4b3ecca12c187766cb7bdc4f18 and 87667cc1391804614e45054a515fd989991da04bfd2d8e9948ff10ccc166b443. Both report source/harness SHAs equal this head. Final report digests: 504579e7a546193931d1c829457ae55626487de2713b1b125c8c44c73e3b5812 and 6c540dcb18f09e57160565a7b0867caa7aa95089aa51c7e1b8b70cdc535bd6d9.
- Go vet/gofmt, shell syntax, real Compose configuration and diff checks pass.
CI38518099663 is still running and is not counted as a completed pass. These fresh executions replace reliance on the older-binary smoke for this bounded profile only: no latest-main, Airbyte/NiFi, source-CDC/full160000-case, performance, HA or EOS certification is implied; #29848 remains open. No blocking finding remains.
|
mo-bot review · Commit: Review completed. Read the assessment. |
XuPeng-SH
left a comment
XuPeng-SH
left a comment
There was a problem hiding this comment.
mo-bot review · mo-bot-local · review
Commit: 2922b56f45e6615454336c7273d2c988195dd361
Assessment: approve
Reviewed the complete eight-file delta and affected callers. Both prior defects are fixed and their regression tests pass. Shared lifecycle ownership, recovery ordering, exact-row/offset checks, failure handling and test scope are reasonable. No actionable blocker remains.
Model: gpt-6-astra / xhigh
Session: 01a12d8f-8e48-7622-ad17-b605f08450ce · slot 1 · generation 0
Validation:
-
Isolation preflight passed: preflight.log and checks/1791962750002637710-2.json.
-
Both checked-in regression tests passed using test-exec: stale-report preflight failure and relative Compose path resolution. Matched focused-regressions.log to checks/1791962797693494096-2.json, exit 0, clean assessed head.
-
Source audit passed: all eight frozen blobs and patch reconciled; shell syntax, Go formatting, diff whitespace and generated TOML/port/socket assertions passed. Evidence: source-audit.log, source-audit.json and checks/1791962877694857736-2.json. Initial audit rejected only a terminal-newline serialization difference; its script and log are retained.
-
Frozen evidence records successful current-head Ubuntu UT, arm64 SCA, coverage, shared build, PROXY/PESSIMISTIC BVT and CI Required. The detailed exact-head review reporting package/race tests and two real Kafka Connect runs was used as scoped corroboration.
-
Reviewed MongoDB helper compatibility, cancellation, teardown, replay prerequisites, NULL versus empty-string handling and independent offset observations. Detailed assessment retained in review-notes.json.
Limitations:
-
local_validation is empty; raw artifacts for the externally reported native runs and race suites were not supplied. Those executions were not independently reproduced or rehashed here.
-
No production SQL path changes or measured performance gains. Harness costs are confined to explicit qualification runs and tests; no benchmark was performed.
-
The linked issue body was unavailable. Approval covers the stated bounded Kafka Connect qualification, not broader connector, scale, HA or exactly-once certification.
Merge Queue Status
This pull request spent 29 minutes 50 seconds in the queue, with no time running CI. ReasonPull request #30025 has been dequeued Pull request from fork cannot be queued. This pull request comes from a fork, and Mergify needs the author's permission to update its branch.
HintYou should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it. Waiting for any of
All conditions
Tick the box to put this pull request back in the merge queue (same as
|
What type of PR is this?
Which issue(s) this PR fixes:
References #29848. This is a bounded qualification and CI-harness update; it does not close the issue.
What this PR does / why we need it:
Issue #29848 already records a historical real Kafka Connect recovery run on MatrixOne source revision
652c643666428f1c434e03369834f2a9bead7c27. This PR makes that recovery path repeatable in the repository and verifies independent sink progress and replay behavior.The
optools/kafka_connect_ci.bash e2e-localprofile runs pinned Kafka 3.9.0 and Debezium Kafka Connect/JDBC 3.1.2.Final against a locally built MatrixOne. It verifies baseline writes, SIGKILL recovery while insert/update/tombstone records accumulate, same-worker recovery without connector re-registration or offset reset, exact typed destination rows, Connect REST offset, Kafka consumer-group offset, and Kafka log-end offset. It then stops the connector before rewinding its fixture offset, verifies controlled replay is idempotent, and checks a no-new-record restart. The oracle preserves SQL NULL versus empty strings and rejects duplicate or missing IDs.The harness shares host-wide port leases and lifecycle helpers with the MongoDB E2E runner. Artifact-redaction and cleanup failures make the run fail, the JDBC sink uses Debezium's
batch.size=1, and transient MatrixOne SQL readiness failures retry until the phase deadline. The new Go seam tests cover SQL, REST, in-process Kafka, Docker helper, cancellation, report, and CLI failure paths, including STOPPING→STOPPED before offset PATCH and an independently lagging REST offset.Validation
Passed at final head
fa339bbb0f727be7aae6a474d55bbb0c5cfad98f(reviewed branch base11914526ba0cd40d471299d8e5e2929909b9e09c):go test ./test/kafkaconnect -count=1— passed (5.900s).go test -race ./test/kafkaconnect -count=1— passed (6.713s).go test ./test/kafkaconnect -count=20— passed (84.511s).go test ./test/kafkaconnect -coverprofile=...— passed (84.6% package statement coverage).go vet ./test/kafkaconnect— passed.bash -n optools/mo_e2e_common.bash optools/mongodb_ci.bash optools/kafka_connect_ci.bash— passed.283/375 = 75.47%modified blocks and284/376 = 75.53%overall, approved above the strict>75%threshold.git diff --check— passed.The local
golangci-lintbinary cannot typecheck this Go 1.27 checkout because its analyzer rejects Go 1.27 export data (version 4 > maximum supported version 2); Go test and vet pass, and repository CI remains authoritative for lint.A separate candidate runtime smoke passed baseline, worker-crash catch-up, controlled replay, and empty-work restart with exact rows and three offsets. It used Kafka 3.9.0 / Debezium JDBC 3.1.2.Final and an owned copy of a prebuilt MatrixOne binary at source revision
4724622326d603e113e2c1ed5a12ced80c6f23f1; it preceded the final hardening and does not certify the corrected final source, current-main compatibility, or the normal checked-in build path. Detailed provenance and limitations are in the task qualification report.This PR does not claim current-main native execution, official Airbyte or NiFi qualification, the full 32-path / 160,000-case matrix, a performance benchmark, or a GitHub CI pass. Airbyte's
LOAD DATA ... NULLIFblocker is tracked separately by #29849 / PR #30022, and the remaining Issue matrix and historical evidence still require separate qualification.Compatibility and performance
This changes local qualification tooling and shared E2E lifecycle helpers; it makes no MatrixOne product API or SQL behavior change. No performance benchmark was run.
Final head SHA:
fa339bbb0f727be7aae6a474d55bbb0c5cfad98f.