Skip to content

Commit 8abc131

Browse files
committed
kvserver: return NodeUnavailable error during range ID allocation
During a split, the request to generate a new range ID for the RHS can race with shutting down the node where the underlying increment is evaluated. Previously, in this case, the range ID allocator would return a custom error (`could not allocate ID; system is draining`), which is not retried by the `DistSender`. This commit replaces this error with a `NodeUnavailableError`, which is how this type of race is handled in other places as well. Informs: #64828 Release note: None
1 parent 75b649c commit 8abc131

File tree

3 files changed

+8
-4
lines changed

3 files changed

+8
-4
lines changed

pkg/kv/kvserver/idalloc/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ go_library(
88
deps = [
99
"//pkg/base",
1010
"//pkg/kv",
11+
"//pkg/kv/kvpb",
1112
"//pkg/roachpb",
1213
"//pkg/util/log",
1314
"//pkg/util/retry",
@@ -28,6 +29,7 @@ go_test(
2829
deps = [
2930
"//pkg/keys",
3031
"//pkg/kv/kvclient/kvcoord",
32+
"//pkg/kv/kvpb",
3133
"//pkg/roachpb",
3234
"//pkg/security/securityassets",
3335
"//pkg/security/securitytest",

pkg/kv/kvserver/idalloc/id_alloc.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"github.com/cockroachdb/cockroach/pkg/base"
1313
"github.com/cockroachdb/cockroach/pkg/kv"
14+
"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
1415
"github.com/cockroachdb/cockroach/pkg/roachpb"
1516
"github.com/cockroachdb/cockroach/pkg/util/log"
1617
"github.com/cockroachdb/cockroach/pkg/util/retry"
@@ -84,7 +85,7 @@ func (ia *Allocator) Allocate(ctx context.Context) (int64, error) {
8485
case id := <-ia.ids:
8586
// when the channel is closed, the zero value is returned.
8687
if id == 0 {
87-
return id, errors.Errorf("could not allocate ID; system is draining")
88+
return id, &kvpb.NodeUnavailableError{}
8889
}
8990
return id, nil
9091
case <-ctx.Done():

pkg/kv/kvserver/idalloc/id_alloc_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414

1515
"github.com/cockroachdb/cockroach/pkg/keys"
1616
"github.com/cockroachdb/cockroach/pkg/kv/kvclient/kvcoord"
17+
"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
1718
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/idalloc"
1819
"github.com/cockroachdb/cockroach/pkg/roachpb"
1920
// Import to set ZoneConfigHook.
@@ -232,9 +233,9 @@ func TestAllocateWithStopper(t *testing.T) {
232233
s, idAlloc := newTestAllocator(t)
233234
s.Stop() // not deferred.
234235

235-
if _, err := idAlloc.Allocate(context.Background()); !testutils.IsError(err, "system is draining") {
236-
t.Errorf("unexpected error: %+v", err)
237-
}
236+
_, err := idAlloc.Allocate(context.Background())
237+
require.Error(t, err)
238+
require.ErrorIs(t, err, &kvpb.NodeUnavailableError{})
238239
}
239240

240241
// TestLostWriteAssertion makes sure that the Allocator performs a best-effort

0 commit comments

Comments
 (0)