Conversation
ti-chi-bot
Bot
added
do-not-merge/needs-linked-issue
release-note
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (16)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughPaused changefeed creation now accepts an optional API or CLI ChangesPaused Changefeed Creation
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant Client
participant CDCAPI
participant Coordinator
participant Etcd
participant Maintainer
participant Downstream
Client->>CDCAPI: Create changefeed with pause=true
CDCAPI->>Coordinator: Save stopped changefeed with bootstrap pending
Coordinator->>Etcd: Persist changefeed metadata
Client->>CDCAPI: Resume changefeed
Maintainer->>Coordinator: Report completed bootstrap
Coordinator->>Etcd: Clear bootstrap pending with FinishInit
Coordinator->>Downstream: Start replication
Suggested reviewers: Merge Risk: ⚪ Minimal · up to Paused changefeeds retain their validated checkpoint and fresh-start behavior until successfully resumed, while normal creation remains unchanged. No actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 17 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches🧪 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. A rabbit taps pause by moonlit light Comment |
|
I think we need to preserve the new-changefeed semantics for a changefeed created with The changefeed is constructed with This matters for MySQL sinks because Could we keep a durable marker for a paused changefeed that has never started, preserve it across coordinator restart, and clear it only after the first maintainer bootstrap succeeds? It would also be good to assert |
ti-chi-bot
Bot
added
size/XXL
ti-chi-bot
Bot
added
lgtm
and removed
needs-1-more-lgtm
[LGTM Timeline notifier]Timeline:
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: asddongmen, nongfushanquan, wk989898 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest-required |
|
/test pull-cdc-pulsar-integration-heavy |
|
@sdojjy: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
4 participants
What problem does this PR solve?
Creating a changefeed immediately schedules replication. Callers need to perform the normal validation and reserve the start position while leaving the new task paused until an explicit resume.
Issue Number: close #6266
What is changed and how it works?
pauseboolean to the v2 create request. Omission andfalseretain automatic startup;truepersistsstoppedand registers the task in the stopped collection without scheduling a maintainer.pause = truein CLI TOML via the internal replica configuration and forward it to the create request. It is a create-only option, not an additional runtime state or nested API replica setting.ddl_ts_v1records for a reused name. Register the case in MySQL light CI G04 and document usage.Check List
Tests
Passed locally:
Full
./coordinator,./coordinator/changefeed, and./coordinator/operatorpackage tests with-p 1 -ldflags='-s -w' -count=1, including first resume/restart, stale owner, failed acknowledgement retry, and metadata compare-and-swap coverage.MySQL
TestGetTableRecoveryInfo_StartTsGreaterThanDDLTsandTestGetTableRecoveryInfo_RemoveDDLTs; local targetedunconvertlint.go test -p 1 -ldflags='-s -w' ./api/v2 ./cmd/cdc/cli ./pkg/config -count=1go test -p 1 -ldflags='-s -w' ./coordinator -run '^Test(CreateChangefeedDoesNotUpdateGCSafepoint|UpdateGCSafepointCallsGCManagerUpdate)$' -count=1Repository-pinned Go/import/shell formatters on changed files, shell syntax checks, diff line-width and log-style checks, and
git diff --check.Not completed locally:
-tags=nextgencould not compile because the disk ran out of space.make integration_test_mysql CASE=changefeed_create_pausedstopped atcheck_third_party_binary: local database test binaries are missing, and the Docker daemon is unavailable. The new integration test is committed and registered in CI but has not been run end-to-end locally.-run TestCreateChangefeedalso hit an existing test's missing global MessageCenter initialization; the focused GC creation tests above initialize their own services and pass.Will it cause performance regression or break compatibility?
The default creation path is unchanged. No new lifecycle state, storage migration, or scheduler abstraction is introduced; existing metadata without the optional bootstrap marker retains its current behavior. Pause affects creation only, and existing paused-task GC TTL limits still apply.
Do you need to update user documentation, design documentation or monitoring documentation?
Added
docs/create-paused-changefeed.mdwith API and CLI TOML examples and lifecycle/GC semantics.Release note
Summary by CodeRabbit
New Features
pauseoption.pauseor setting it tofalseretains automatic startup.Documentation
Tests