Skip to content

Commit

Permalink
kvserver: remove StoreBenignError
Browse files Browse the repository at this point in the history
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
  • Loading branch information
XiaochenCui committed Sep 10, 2024
1 parent fa9c052 commit dd32bc1
Show file tree
Hide file tree
Showing 4 changed files with 1 addition and 56 deletions.
5 changes: 1 addition & 4 deletions pkg/kv/kvserver/benignerror/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library")

go_library(
name = "benignerror",
srcs = [
"benign_error.go",
"store_benign_error.go",
],
srcs = ["benign_error.go"],
importpath = "github.com/cockroachdb/cockroach/pkg/kv/kvserver/benignerror",
visibility = ["//visibility:public"],
deps = ["@com_github_cockroachdb_errors//:errors"],
Expand Down
32 changes: 0 additions & 32 deletions pkg/kv/kvserver/benignerror/store_benign_error.go

This file was deleted.

5 changes: 0 additions & 5 deletions pkg/kv/kvserver/queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -1175,17 +1175,12 @@ func (bq *baseQueue) finishProcessingReplica(
// Handle failures.
if err != nil {
benign := benignerror.IsBenign(err)
storeBenign := benignerror.IsStoreBenign(err)

// Increment failures metric.
//
// TODO(tschottdorf): once we start asserting zero failures in tests
// (and production), move benign failures into a dedicated category.
bq.failures.Inc(1)
if storeBenign {
bq.storeFailures.Inc(1)
requeue = true
}

// Determine whether a failure is a purgatory error. If it is, add
// the failing replica to purgatory. Note that even if the item was
Expand Down
15 changes: 0 additions & 15 deletions pkg/kv/kvserver/queue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/config"
"github.com/cockroachdb/cockroach/pkg/config/zonepb"
"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/benignerror"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverbase"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/settings"
Expand Down Expand Up @@ -1564,18 +1563,4 @@ func TestBaseQueueRequeue(t *testing.T) {
bq.maybeAdd(ctx, r1, hlc.ClockTimestamp{})
assertShouldQueueCount(6)
assertProcessedAndProcessing(2, 0)

// Reset shouldQueueCount so we actually process the replica. Then return
// a StoreBenign error. It should requeue the replica.
atomic.StoreInt64(&shouldQueueCount, 0)
pQueue.err = benignerror.NewStoreBenign(errors.New("test"))
bq.maybeAdd(ctx, r1, hlc.ClockTimestamp{})
assertShouldQueueCount(1)
assertProcessedAndProcessing(2, 1)
// Let the first processing attempt finish. It should requeue.
pQueue.processBlocker <- struct{}{}
assertProcessedAndProcessing(3, 1)
pQueue.err = nil
pQueue.processBlocker <- struct{}{}
assertProcessedAndProcessing(4, 0)
}

0 comments on commit dd32bc1

Please sign in to comment.