Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

disable the config enable-partition-separator #11979

Open
lidezhu opened this issue Jan 6, 2025 · 7 comments
Open

disable the config enable-partition-separator #11979

lidezhu opened this issue Jan 6, 2025 · 7 comments
Labels
affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. affects-8.5 This bug affects the 8.5.x(LTS) versions. area/ticdc Issues or PRs related to TiCDC. report/customer Customers have encountered this bug. type/bug The issue is confirmed as a bug.

Comments

@lidezhu
Copy link
Collaborator

lidezhu commented Jan 6, 2025

What did you do?

  1. create a changefeed with sink type set to storage services and enable-partition-separator set to false;
  2. create a partition table with multiple partitions and write data;

What did you expect to see?

All rows are written to storage services;

What did you see instead?

Some rows are lost;

Versions of the cluster

Upstream TiDB cluster version (execute SELECT tidb_version(); in a MySQL client):

(paste TiDB cluster version here)

Upstream TiKV version (execute tikv-server --version):

(paste TiKV version here)

TiCDC version (execute cdc version):

(paste TiCDC version here)
@lidezhu lidezhu added area/ticdc Issues or PRs related to TiCDC. type/bug The issue is confirmed as a bug. affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. affects-8.5 This bug affects the 8.5.x(LTS) versions. labels Jan 6, 2025
@ti-chi-bot ti-chi-bot bot added the report/customer Customers have encountered this bug. label Jan 6, 2025
@lidezhu
Copy link
Collaborator Author

lidezhu commented Jan 7, 2025

Problems introduced by this pr: #8617

@lidezhu
Copy link
Collaborator Author

lidezhu commented Jan 8, 2025

Because different partitions may be on different cdc nodes, it is hard to coordinate between them. So we must remove the config enable-partition-separator and make it always true.

@lidezhu
Copy link
Collaborator Author

lidezhu commented Jan 10, 2025

When the sink type is storage, for partition table, the default file path for CDC on remote storage is {scheme}://{prefix}/{schema}/{table}/{table-version-separator}/{partition-separator}/{date-separator}/CDC{num}.{extension}. And the {partition-separator} segment represents the physical ID of the partition.

However, when the configuration option enable-partition-separator is set to false, the {partition-separator} segment is excluded from the file path, resulting in {scheme}://{prefix}/{schema}/{table}/{table-version-separator}/{date-separator}/CDC{num}.{extension}. In this case, different partitions of the same table write to the same directory, and the {num} part becomes the only differentiator for CDC files.

If different partitions are handled by separate CDC nodes, maintaining a globally unique {num} counter to avoid filename conflicts becomes extremely complex. This issue arises when such conflicts lead to file overwrites and data loss. To address this problem, it is recommended to remove the enable-partition-separator configuration option, ensuring that data from different partitions is always written to separate folders to avoids file name conflicts and prevents data loss.
PTAL @benmeadowcroft

@lidezhu lidezhu changed the title data of partition table may lost when sink type is storage services and enable-partition-separator is false disable the config enable-partition-separator Jan 10, 2025
@benmeadowcroft
Copy link

@lidezhu given that we may have users that have configured the solution this way already, we should go through the deprecation and removal process for the setting (as we should do with other configuration changes). To do that we would first have to issue warnings when we detect this setting being used, before we make a change to remove the setting in a later release. This would give an opportunity for users to correct their configurations prior to a subsequent release beginning to raise invalid parameter errors when they try and use the feature.

@lidezhu
Copy link
Collaborator Author

lidezhu commented Jan 14, 2025

OK, when this setting is detected and a warning is issued, should we proceed with the configuration value provided by the user, or should we disregard their input and forcibly set the configuration to true? @benmeadowcroft

@flowbehappy
Copy link
Collaborator

flowbehappy commented Jan 14, 2025

@benmeadowcroft Do you have a specific suggestion about the versions and the related actions? For example:

  • v9.0(DMR) ~ v9.3(LTS): Show a warning if a user set this parameter incorrectly, but keep the incorrect setting value anyway
  • v9.4(DMR): Ignore users' incorrect setting and show a warning

@benmeadowcroft
Copy link

Yes, I think that the following would work:

  • From v9.0 (DMR) to v9.3(LTS): Mark as deprecated in the documentation. Show a warning if the user sets this parameter explicitly (whether "correct" or "incorrect" and inform them it is deprecated and to remove the setting), but keep the behavior the same as currently
  • v9.4(DMR) to next LTS: Ignore users' incorrect setting and show a warning
  • Subsequent DMR: Remove the setting entirely and if present in the customers configuration return an error

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. affects-8.5 This bug affects the 8.5.x(LTS) versions. area/ticdc Issues or PRs related to TiCDC. report/customer Customers have encountered this bug. type/bug The issue is confirmed as a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants