Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
ti-chi-bot
Bot
added
missing-translation-status
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds TiDB Cloud Data Pipeline documentation. It covers navigation, pipeline operation, external-stage configuration, support details, FAQs, and setup procedures for Premium and Essential instances. ChangesData Pipeline Documentation
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~15 minutes Change: Other Merge Risk: 🟡 Moderate · up to The setup guides can lead users to grant broader storage access than intended, and non-AWS Essential users may be unable to use the documented Role ARN path. These issues should be corrected before merge. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: pingcap/docs/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 41262998-272e-4187-8556-a224f9c9df77
📒 Files selected for processing (7)
TOC-tidb-cloud-premium.mdtidb-cloud/data-pipeline/data-pipeline-overview.mdtidb-cloud/data-pipeline/lake-data-pipeline-configure-external-stage.mdtidb-cloud/data-pipeline/lake-data-pipeline-faq.mdtidb-cloud/data-pipeline/lake-data-pipeline-support-matrix.mdtidb-cloud/data-pipeline/setup-lake-data-pipeline-for-essential.mdtidb-cloud/data-pipeline/setup-lake-data-pipeline-for-premium.md
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.
| 2. Read the warning and confirm the operation. Deleting a data pipeline: | ||
|
|
||
| - Immediately stops all data replication. | ||
| - Removes the TiDB Cloud Lake data source and integration task associated with the pipeline. |
There was a problem hiding this comment.
Could we avoid stating this as unconditional? The delete workflow can finish while Lake cleanup is skipped when Lake refuses the request (for example, HTTP 403); the service explicitly leaves the Lake task/data source for manual cleanup in that case. Please say “attempts to remove” and mention that resources may remain if Lake refuses the request.
|
|
||
| - The TiDB Cloud Lake warehouse must be in the **same region** as your {{{ .premium }}} instance. | ||
| - Only tables with a **primary key** can be replicated incrementally. If a table in the sync scope has no primary key, its incremental replication fails and an error is reported for that table. | ||
| - You can create up to 100 data pipelines per {{{ .premium }}} instance. |
There was a problem hiding this comment.
The service code does not enforce “100 data pipelines per Premium instance”. The only matching quota is bizChangefeedPerClusterQuota = 100; a full-only pipeline has no changefeed at all, while an incremental pipeline consumes one. Please either cite the separate product quota or describe the actual changefeed/cluster limit, otherwise this restriction is not aligned with the implementation.
| - **Full Data + Incremental Data** (default): exports a full snapshot of the selected source data, and then continuously replicates row changes. This is the recommended mode for ongoing synchronization. | ||
| - **Full Data**: exports a one-time full snapshot of the selected source data only. No incremental data is replicated, and changes made on the source after the snapshot are ignored. | ||
|
|
||
| 2. **Sync Interval**: the interval at which the data pipeline scans for new data. The default is `5 minutes`. Shorter intervals reduce data latency but increase the number of API calls to cloud storage. |
There was a problem hiding this comment.
This is not the interval that the service directly uses to scan Lake. dataflow-service-ng treats the request as an end-to-end latency budget and splits it between TiCDC flush and Lake polling (producer 2–600s, consumer 5–300s); when SQS is configured, the consumer poll interval is set to zero. I also could not find a service default of 5 minutes. Please document the end-to-end semantics and source the default from the actual UI/API contract before stating it here.
| ## Restrictions | ||
|
|
||
| - The TiDB Cloud Lake warehouse must be in the **same region** as your {{{ .premium }}} instance. | ||
| - Only tables with a **primary key** can be replicated incrementally. If a table in the sync scope has no primary key, its incremental replication fails and an error is reported for that table. |
There was a problem hiding this comment.
Please qualify the no-primary-key behavior. The pipeline does not reject these tables at creation: source validation returns them in the deny list, the snapshot path still counts/exports the selected set, and the generated TiCDC changefeed uses IGNORE_NOT_SUPPORT_TABLE. Whether a per-table error is shown is downstream behavior, so “incremental replication fails and an error is reported” needs evidence or more precise wording.
|
|
||
| ## Step 1: Create an OSS bucket | ||
|
|
||
| > 💡 If you already have an OSS bucket ready, skip this step — just make sure the bucket region matches the region of your TiDB Cloud instance. |
There was a problem hiding this comment.
The same-region requirement is not enforced for Alibaba OSS by dataflow-service-ng: its provider tests explicitly accept an OSS region unrelated to the cluster, and the region check is only applied to the AWS path. Please scope this sentence to AWS, or cite the separate Lake/product constraint if OSS is intentionally restricted there.
|
|
||
| ## DDL support summary | ||
|
|
||
| | DDL pattern | Status | Behavior / symptom | |
There was a problem hiding this comment.
This support matrix cannot be derived from the Data Pipeline implementation as written. The service configures TiCDC CANAL_JSON/content-compatible output, IGNORE_NOT_SUPPORT_TABLE, and Lake AllowDelete; it has no DDL/type compatibility table or translation for the listed operations and types. Please cite the tested TiCDC/Lake version and source for these claims, or label this as a separately validated compatibility matrix rather than an implementation guarantee.
| | `DROP COLUMN` | ✅ | | | ||
| | `ADD INDEX` / `DROP INDEX` | ✅ | | | ||
| | `RENAME COLUMN` | ✅ | | | ||
| | `DROP TABLE` | N/A | Destination table is kept in Lake. | |
There was a problem hiding this comment.
This behavior conflicts with the webapi/bend-cdc implementation/docs used by the manual Lake integration: bend-cdc/docs/tidb-column-ddl.md says DROP TABLE, TRUNCATE, and RENAME TABLE are unsupported and block subsequent consumption for the table, rather than being silently ignored while the destination keeps/splits data. Please reconcile this matrix with the deployed engine/version and state whether these events stop one table or are intentionally skipped.
| - **Table Rules**: `*.*` to sync all exported tables. | ||
| - **Changefeed S3 Prefix**: `<prefix>/incremental/`. | ||
| - **Dumpling S3 Prefix**: `<prefix>/snapshot/`. | ||
| - **Poll Interval**: default or as needed. A shorter interval reduces data latency but increases Lake hosting cost. |
There was a problem hiding this comment.
Please state the actual defaults here. In webapi/bend-cdc, the TiDB manager defaults PollInterval to 60 seconds and MergeInterval to 30 seconds when the API leaves them unset; these controls have different latency/cost effects. “Default or as needed” is too vague for a manual procedure and can make the documented behavior diverge from the deployed worker.
|
|
||
| In the default mode, the Data Pipeline relies on two independent polling intervals — one on the Changefeed side (which periodically flushes incremental data to the external stage) and one on the Lake side (which periodically scans the external stage for new data). Because these two intervals do not coordinate, the effective end-to-end latency is higher than either interval alone. | ||
|
|
||
| In event-driven mode, the Changefeed still flushes data to the external stage on its configured interval, but each flush also triggers an **S3 event notification** to an SQS queue. Lake subscribes to this queue and loads new data as soon as the notification arrives, eliminating the additional latency caused by its own polling interval. |
There was a problem hiding this comment.
This description is not true for the Essential/manual path backed by webapi/bend-cdc: SQS is only a latency optimization that wakes an authoritative listing; the manager still runs scheduled polling and reports sqs+polling. Please scope this to the native dataflow-service-ng path (if that path really uses zero polling), or describe the two products separately so Essential users do not assume SQS replaces polling.
|
|
||
| > **Note:** | ||
| > | ||
| > Only tables with a primary key can be replicated incrementally. Tables that lack a primary key are listed separately in the **Filter results** panel, and their incremental replication fails. Add a primary key to these tables before you create the data pipeline, or exclude them with filter rules such as `"!test.tbl1"`. |
There was a problem hiding this comment.
This now contradicts the restriction above: line 23 correctly says that no-primary-key tables are skipped for incremental replication, but this note still says their replication “fails”. The Data Pipeline creates the TiCDC changefeed with IGNORE_NOT_SUPPORT_TABLE, so please use the same “skipped/excluded from incremental replication” wording here; otherwise users receive two incompatible outcomes in one setup guide.
| - **Role ARN**: the ARN from [Step 1](#step-1-create-the-role-with-export-cloudformation), or **Access Key ID** / **Secret Access Key** from [Use an Access Key](#use-an-access-key). | ||
| - **S3 Bucket Name**: the bucket name only (for example, `my-datapipeline-bucket`, not the full URI). | ||
| - **S3 Region**: the same region as your Essential instance. | ||
| 4. **SQS Queue URL** is optional. |
There was a problem hiding this comment.
The manual flow exposes an optional queue URL but never gives Essential users the required queue setup: S3 bucket notification, queue policy, and ReceiveMessage/DeleteMessage/GetQueueAttributes permissions on the same Role ARN or access key. webapi/bend-cdc constructs its SQS consumer from those credentials and silently falls back to periodic listing when the queue is inaccessible, so a user can think event-driven ingestion is enabled when it is not. Please add the setup steps or link directly to the corresponding AWS SQS procedure.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: pingcap/docs/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Advanced
Run ID: cf7bbcf8-dd5e-49f9-9717-9d9985c009b9
📒 Files selected for processing (7)
TOC-tidb-cloud-premium.mdtidb-cloud/data-pipeline/data-pipeline-overview.mdtidb-cloud/data-pipeline/lake-data-pipeline-configure-external-stage.mdtidb-cloud/data-pipeline/lake-data-pipeline-faq.mdtidb-cloud/data-pipeline/lake-data-pipeline-support-matrix.mdtidb-cloud/data-pipeline/setup-lake-data-pipeline-for-essential.mdtidb-cloud/data-pipeline/setup-lake-data-pipeline-for-premium.md
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.
|
@sdojjy: adding LGTM is restricted to approvers and reviewers in OWNERS files. DetailsIn response to this: Instructions 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. |
lilin90
added
lake
ti-chi-bot
Bot
removed
the
missing-translation-status
lilin90
changed the title
|
@ginkgoch: The following test 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
First-time contributors' checklist
What is changed, added, or deleted? (Required)
Add data pipeline documents
Which TiDB version(s) do your changes apply to? (Required)
Tips for choosing the affected version(s):
By default, CHOOSE MASTER ONLY so your changes will be applied to the next TiDB major or minor releases. If your PR involves a product feature behavior change or a compatibility change, CHOOSE THE AFFECTED RELEASE BRANCH(ES) AND MASTER.
For details, see tips for choosing the affected versions.
What is the related PR or file link(s)?
AI agent involvement
Do your changes match any of the following descriptions?
Summary by CodeRabbit