Improve note. - #23362
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 |
|
Hi @ginkgoch. Thanks for your PR. I'm waiting for a pingcap member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
📝 WalkthroughWalkthroughThe documentation updates ChangesS3 multipart size guidance
Estimated code review effort: 1 (Trivial) | ~2 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ 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. Comment |
| > **Note:** | ||
| > | ||
| > This configuration is introduced to address backup failures caused by S3 rate limiting. This problem has been fixed by [refining the backup data storage structure](/br/br-snapshot-architecture.md#structure-of-backup-files). Therefore, this configuration is deprecated from v6.1.1 and is no longer recommended. | ||
| > This configuration was introduced to address backup failures caused by S3 rate limiting. Since this issue has been resolved by [refining the backup data storage structure](/br/br-snapshot-architecture.md#structure-of-backup-files), from v6.1.1 onwards, configure this setting with caution. Setting it too large may cause large parts to fail or time out during network jitter. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Scope the resolution claim to large-cluster backups.
The supplied v6.1.1 release note describes fixing S3 rate-limiting failures specifically for large-cluster backups, not all such failures. The current wording overstates the fix.
Suggested replacement
| > This configuration was introduced to address backup failures caused by S3 rate limiting. Since this issue has been resolved by [refining the backup data storage structure](/br/br-snapshot-architecture.md#structure-of-backup-files), from v6.1.1 onwards, configure this setting with caution. Setting it too large may cause large parts to fail or time out during network jitter. | |
| > This configuration was introduced to address backup failures caused by S3 rate limiting. Since the backup failure caused by S3 rate limiting in large cluster backups was resolved by [refining the backup data storage structure](/br/br-snapshot-architecture.md#structure-of-backup-files), from v6.1.1 onwards, configure this setting with caution. Setting it too large may cause large parts to fail or time out during network jitter. |
Source: Path instructions
First-time contributors' checklist
What is changed, added or deleted? (Required)
Which TiDB version(s) do your changes apply to? (Required)
In real-world scenarios, a small percentage of customers still exceed their S3 API quotas, making this configuration still valuable. Since there is no other workaround and the configuration will not be removed, this update replaces the word "deprecated" with "configure with caution." This aims to reduce the hesitation or mental burden for customers who genuinely need to use it to resolve their issues.
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