Skip to content

Improve note. - #23362

Open
ginkgoch wants to merge 2 commits into
pingcap:release-8.5from
ginkgoch:improve-br-note-s3-multipart-size
Open

Improve note.#23362
ginkgoch wants to merge 2 commits into
pingcap:release-8.5from
ginkgoch:improve-br-note-s3-multipart-size

Conversation

@ginkgoch

@ginkgoch ginkgoch commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

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.

  • master (the latest development version)
  • v9.0 (TiDB 9.0 versions)
  • v8.5 (TiDB 8.5 versions)
  • v8.1 (TiDB 8.1 versions)
  • v7.5 (TiDB 7.5 versions)
  • v7.1 (TiDB 7.1 versions)
  • v6.5 (TiDB 6.5 versions)

What is the related PR or file link(s)?

  • This PR is translated from:
  • Other reference link(s):

AI agent involvement

  • The changes in this PR were primarily made by an AI agent on behalf of the PR author.

Do your changes match any of the following descriptions?

  • Delete files
  • Change aliases
  • Need modification after applied to another branch
  • Might cause conflicts after applied to another branch

Summary by CodeRabbit

  • Documentation
    • Updated guidance for the S3 multipart backup size setting.
    • Clarified that the setting was introduced to help with S3 rate-limiting-related backup failures, and is no longer considered deprecated from v6.1.1 onward.
    • Reiterated that configuring values too large may still lead to multipart part failures or timeouts under network jitter.

@ti-chi-bot

ti-chi-bot Bot commented Jul 27, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign oreoxmt for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot Bot added contribution This PR is from a community contributor. first-time-contributor Indicates that the PR was contributed by an external member and is a first-time contributor. labels Jul 27, 2026
@ti-chi-bot

ti-chi-bot Bot commented Jul 27, 2026

Copy link
Copy Markdown

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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

@ti-chi-bot ti-chi-bot Bot added needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. missing-translation-status This PR does not have translation status info. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 27, 2026
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The documentation updates s3-multi-part-size guidance, replacing its deprecation notice with the setting’s original purpose, the storage structure refinement, and cautions about oversized parts failing or timing out under network jitter.

Changes

S3 multipart size guidance

Layer / File(s) Summary
Update multipart size documentation
tikv-configuration-file.md
The configuration note describes the original S3 rate-limiting purpose, the storage structure fix, and caution when using large values.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Suggested reviewers: qiancai

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The required change-summary section is blank, so the template is not fully filled out. Add a concise change summary in the required section and keep the version selection and reference links filled out.
Title check ❓ Inconclusive The title is vague and generic, so it does not identify the documentation change. Rename it to a specific title like "Update S3 multipart-size note" so readers know what changed.
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9fc93a5b-a1af-4924-bb64-6ae4277804cb

📥 Commits

Reviewing files that changed from the base of the PR and between e9b2277 and 959ba85.

📒 Files selected for processing (1)
  • tikv-configuration-file.md

> **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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 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
Suggested change
> 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

@ginkgoch
ginkgoch requested review from 3pointer and YuJuncen July 27, 2026 10:02
@qiancai qiancai added the needs-cherry-pick-master Should cherry pick this PR to master branch. label Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contribution This PR is from a community contributor. first-time-contributor Indicates that the PR was contributed by an external member and is a first-time contributor. missing-translation-status This PR does not have translation status info. needs-cherry-pick-master Should cherry pick this PR to master branch. needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants