Skip to content

fix cdc watermark add failure handling#25925

Merged
mergify[bot] merged 3 commits into
matrixorigin:mainfrom
jiangxinmeng1:fix-cdc-watermark-add-failure
Jul 21, 2026
Merged

fix cdc watermark add failure handling#25925
mergify[bot] merged 3 commits into
matrixorigin:mainfrom
jiangxinmeng1:fix-cdc-watermark-add-failure

Conversation

@jiangxinmeng1

@jiangxinmeng1 jiangxinmeng1 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

issue #25921

What this PR does / why we need it:

  • Updated pkg/cdc/watermark_updater.go: execAddWM now returns immediately when the
    catalog INSERT fails, so it no longer writes the watermark into cacheCommitted. The
    existing onJobs cleanup path then propagates the error to the waiting job.

    • Updated pkg/cdc/watermark_updater_test.go: added
      TestAuditAddWatermarkFailureIsReturnedAndNotCached, covering the case where SELECT
      returns no watermark row, INSERT fails with an injected error, the job receives that
      error, and the failed watermark is not cached.

@qodo-code-review

qodo-code-review Bot commented Jul 21, 2026

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@mergify mergify Bot added the kind/bug Something isn't working label Jul 21, 2026
@matrix-meow matrix-meow added the size/S Denotes a PR that changes [10,99] lines label Jul 21, 2026
gouhongshen
gouhongshen approved these changes Jul 21, 2026
View reviewed changes

@gouhongshen gouhongshen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex automated review

The INSERT error now returns before cache mutation or success notification; deferred cleanup propagates the original error and clears affected jobs. The regression test covers both error propagation and cache integrity. Existing PR discussions contained no substantive prior findings. CI is green; local tests were blocked by missing prebuilt CGo dependencies.

@mergify

mergify Bot commented Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Queued — the merge queue status continues in this comment ↓.

@mergify

mergify Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

queue

☑️ Command queue ignored because it is already running from a previous command.

@mergify

mergify Bot commented Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • ✅ Entered queue — 2026-07-21 06:43 UTC · Rule: main · triggered by @jiangxinmeng1 with the merge queue checkbox
  • 🚫 Left the queue — 2026-07-21 06:53 UTC · at b3f8365f99aacbdb7aad2a477c8ba99585ab451d

This pull request spent 9 minutes 49 seconds in the queue, with no time running CI.

Reason

The pull request can't be updated

For security reasons, Mergify can't update this pull request. Try updating locally.
GitHub response: refusing to allow a GitHub App to create or update workflow .github/workflows/iceberg-connector.yml without workflows permission

Hint

You should update or rebase your pull request manually. If you do, this pull request will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue, you can requeue the pull request, without updating it, by posting a @mergifyio queue comment.

Requeued — the merge queue status continues in this comment ↓.

@mergify mergify Bot removed the dequeued label Jul 21, 2026
@mergify

mergify Bot commented Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • ✅ Entered queue — 2026-07-21 08:46 UTC · Rule: main · triggered by @jiangxinmeng1 with the merge queue checkbox
  • ✅ Checks passed · in-place
  • ✅ Merged — 2026-07-21 10:17 UTC · at f8878973e944c31e4508d49cb1d47d108914c2bd · squash

This pull request spent 1 hour 30 minutes 39 seconds in the queue, including 57 minutes 54 seconds running CI.

Required conditions to merge
  • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
  • github-review-approved [🛡 GitHub branch protection] (documentation)
  • github-review-decision = APPROVED [🛡 GitHub branch protection] (documentation)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
    • check-neutral = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
    • check-skipped = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
    • check-neutral = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
    • check-skipped = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
    • check-neutral = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
    • check-skipped = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone CI / UT Test on Ubuntu/x86
    • check-neutral = Matrixone CI / UT Test on Ubuntu/x86
    • check-skipped = Matrixone CI / UT Test on Ubuntu/x86
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Utils CI / Coverage
    • check-neutral = Matrixone Utils CI / Coverage
    • check-skipped = Matrixone Utils CI / Coverage
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone CI / SCA Test on Linux/arm64
    • check-neutral = Matrixone CI / SCA Test on Linux/arm64
    • check-skipped = Matrixone CI / SCA Test on Linux/arm64

@mergify mergify Bot added the queued label Jul 21, 2026
@mergify
mergify Bot merged commit 8480da0 into matrixorigin:main Jul 21, 2026
30 checks passed
@mergify mergify Bot removed the queued label Jul 21, 2026
@jiangxinmeng1 jiangxinmeng1 mentioned this pull request Jul 23, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gouhongshen gouhongshen gouhongshen approved these changes

Assignees

No one assigned

Labels

kind/bug Something isn't working size/S Denotes a PR that changes [10,99] lines

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jiangxinmeng1 @gouhongshen @matrix-meow