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

kvserver: remove StoreBenignError #130308

Closed
wants to merge 1 commit into from

Conversation

XiaochenCui
Copy link
Contributor

@XiaochenCui XiaochenCui commented Sep 8, 2024

Before commit 3f0b37a, the StoreBenignError is used to handle pebble.ErrSnapshotExcised. As the latter has been removed from pebble, we don't need StoreBenignError anymore.

This commit does the following:

  • Remove type "StoreBenignError".
  • Remove the related increase action on counter "storeFailures".
  • Update related tests "TestBaseQueueRequeue".

Fixes: #129941

Release note: None

Deprecation of benignerror.NewStoreBenign :
3f0b37a#diff-7052dadbb62a54f17fe2ab2ea16258a8e225f89e0670342b00c82ea80935bee8

Deprecation of pebble.ErrSnapshotExcised:
cockroachdb/pebble@1d7852b#diff-0bdcb7129d4d3bd87c0a2d883a50d310991a7bc4ae4b8ab311d63072bde219a8

Copy link

blathers-crl bot commented Sep 8, 2024

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

I have added a few people who may be able to assist in reviewing:

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

@blathers-crl blathers-crl bot added O-community Originated from the community X-blathers-triaged blathers was able to find an owner labels Sep 8, 2024
@blathers-crl blathers-crl bot requested a review from tbg September 8, 2024 23:09
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@XiaochenCui XiaochenCui marked this pull request as ready for review September 8, 2024 23:11
@XiaochenCui XiaochenCui requested a review from a team as a code owner September 8, 2024 23:11
Copy link
Member

@tbg tbg left a comment

Choose a reason for hiding this comment

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

Thank you! Small point about the behavior change that would need to be addressed, but looks good otherwise.

pkg/kv/kvserver/queue.go Outdated Show resolved Hide resolved
Copy link

blathers-crl bot commented Sep 9, 2024

Thank you for updating your pull request.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

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

@XiaochenCui XiaochenCui marked this pull request as draft September 9, 2024 19:16
@XiaochenCui
Copy link
Contributor Author

test TestBaseQueueRequeue failed, going to review the queue logic and update the test later

Before commit 3f0b37a, the
StoreBenignError is used to handle pebble.ErrSnapshotExcised. As the
latter has been removed from pebble, we don't need StoreBenignError
anymore.

This commit does the following:

- Remove type "StoreBenignError".
- Remove the related increase action on counter "storeFailures".
- Update related tests "TestBaseQueueRequeue".

Fixes: cockroachdb#129941

Release note: None
@XiaochenCui XiaochenCui marked this pull request as ready for review September 10, 2024 23:09
@tbg tbg self-requested a review December 13, 2024 07:14
@tbg
Copy link
Member

tbg commented Jan 16, 2025

Thank you! I'm rebasing this over in #139150 (had some difficulties pushing to the fork).

@tbg tbg closed this Jan 21, 2025
craig bot pushed a commit that referenced this pull request Jan 21, 2025
139150: kvserver: remove StoreBenignError r=tbg a=tbg

Before commit 3f0b37a, the StoreBenignError is used to handle pebble.ErrSnapshotExcised. As the latter has been removed from pebble, we don't need StoreBenignError anymore.

This commit does the following:

- Remove type "StoreBenignError".
- Remove the related increase action on counter "storeFailures".
- Update related tests "TestBaseQueueRequeue".

Fixes: #129941

Closes: #130308

Release note: None

139280: roachtest: adding backup/restore tests for minio r=sravotto a=sravotto

Introducing a test to verify that we can backup and restore into a Minio Object Store cluster, using S3 API.

Fixes: #139272

Release note: None

139333: roachtest: only run 30 node backfill tests in full ac mode r=andrewbaptist a=andrewbaptist

In the non-full AC modes, a node can OOM during the fill period and the test will fail. This impacts the perturbation/metamorphic/backfill test.

Fixes: #139302
Informs: #139319

Release note: None

139475: rangefeed: fix test logging r=tbg a=stevendanna

The logging didn't actually print the value as it seemed to intend.

Informs #119340

Release note: None

Co-authored-by: XiaochenCui <[email protected]>
Co-authored-by: Tobias Grieger <[email protected]>
Co-authored-by: Silvano Ravotto <[email protected]>
Co-authored-by: Andrew Baptist <[email protected]>
Co-authored-by: Steven Danna <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-community Originated from the community X-blathers-triaged blathers was able to find an owner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kvserver: remove StoreBenignError
3 participants