Skip to content

Conversation

@msbutler
Copy link
Collaborator

Backport 1/1 commits from #156496 on behalf of @msbutler.


Disabling the client side retry token bucket has made s3 backups at scale more stable.

Epic: none

Release note: none


Release justification:

Disabling the client side retry token bucket has made s3 backups at scale more
stable.

Epic: none

Release note: none
@msbutler msbutler requested a review from a team as a code owner October 30, 2025 14:46
@msbutler msbutler force-pushed the blathers/backport-release-25.2-156496 branch from 04f73b8 to edce497 Compare October 30, 2025 14:46
@msbutler msbutler requested review from kev-cao and removed request for a team October 30, 2025 14:46
@blathers-crl blathers-crl bot added blathers-backport This is a backport that Blathers created automatically. O-robot Originated from a bot. labels Oct 30, 2025
@blathers-crl blathers-crl bot requested a review from jeffswenson October 30, 2025 14:46
@blathers-crl
Copy link

blathers-crl bot commented Oct 30, 2025

Thanks for opening a backport.

Before merging, please confirm that it falls into one of the following categories (select one):

  • Non-production code changes. Includes test-only changes, build system changes, etc.
  • Fixes for serious issues. Defined in the policy as correctness, stability, or security issues, data corruption/loss, significant performance regressions, breaking working and widely used functionality, or an inability to detect and debug production issues.
  • Other approved changes. These changes must be gated behind a disabled-by-default feature flag unless there is a strong justification not to.

Add a brief release justification to the PR description explaining your selection.

Also, confirm that the change does not break backward compatibility and complies with all aspects of the backport policy.

All backports must be reviewed by the TL and EM for the owning area.

@blathers-crl blathers-crl bot added backport Label PR's that are backports to older release branches T-disaster-recovery labels Oct 30, 2025
@blathers-crl
Copy link

blathers-crl bot commented Oct 30, 2025

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@blathers-crl
Copy link

blathers-crl bot commented Oct 30, 2025

✅ PR #156542 is compliant with backport policy

Confidence: high
Critical bug criteria met: [Bugs that can cause the DB to return incorrect results or result in suboptimal performance]
Feature flag detected: Yes
Backward compatible: true
Explanation: The PR introduces a change in the pkg/cloud/amazon/s3_storage.go file where a default setting (cloudstorage.s3.client_retry_token_bucket.enabled) is being modified. Originally set to true, it is now changed to false, thus disabling the feature by default. This aligns with the backport policy requirement that new features or changes must be gated behind a disabled feature flag, especially if not addressing a critical bug. This implies a way to control feature behavior at runtime and ensures the feature can be safely backported without introducing unexpected behaviors. The change was made to stabilize s3 backups at scale as mentioned in the commit message, indicating the potential correction of suboptimal performance, satisfying one criterion of critical bugs—though the primary compliance here is through proper feature flag usage. There is also no indication of breaking backward compatibility as the PR deals with a modification of an existing setting to a safer default state.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@jeffswenson jeffswenson left a comment

Choose a reason for hiding this comment

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

LGTM

@msbutler msbutler self-assigned this Oct 30, 2025
@msbutler msbutler merged commit 1e2478e into cockroachdb:release-25.2 Oct 30, 2025
15 of 16 checks passed
@msbutler msbutler deleted the blathers/backport-release-25.2-156496 branch October 30, 2025 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport Label PR's that are backports to older release branches blathers-backport This is a backport that Blathers created automatically. O-robot Originated from a bot. T-disaster-recovery target-release-25.2.9

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants