errorutil: moved kv-specific errors out of errorutil

Errors for missing store and node descriptors have been moved from the errorutil package to the kvpb package, because the errorutil package is a low-level package to aid in working with errors in general. It should not contain facilities for creating specific errors as this muddles the package and can lead to import cycles. Release note: None
cockroachdb · Sep 14, 2023 · 5460e1e · 5460e1e
1 parent 82ea947
commit 5460e1e
Show file tree

Hide file tree

Showing 16 changed files with 71 additions and 59 deletions.
diff --git a/pkg/ccl/kvccl/kvfollowerreadsccl/BUILD.bazel b/pkg/ccl/kvccl/kvfollowerreadsccl/BUILD.bazel
@@ -74,7 +74,6 @@ go_test(
         "//pkg/testutils/sqlutils",
         "//pkg/testutils/testcluster",
         "//pkg/util",
-        "//pkg/util/errorutil",
         "//pkg/util/hlc",
         "//pkg/util/leaktest",
         "//pkg/util/log",

diff --git a/pkg/ccl/kvccl/kvfollowerreadsccl/followerreads_test.go b/pkg/ccl/kvccl/kvfollowerreadsccl/followerreads_test.go
@@ -40,7 +40,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
 	"github.com/cockroachdb/cockroach/pkg/util"
-	"github.com/cockroachdb/cockroach/pkg/util/errorutil"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -497,15 +496,15 @@ func (s mockNodeStore) GetNodeDescriptor(id roachpb.NodeID) (*roachpb.NodeDescri
 			return desc, nil
 		}
 	}
-	return nil, errorutil.NewNodeNotFoundError(id)
+	return nil, kvpb.NewNodeDescNotFoundError(id)
 }
 
 func (s mockNodeStore) GetNodeDescriptorCount() int {
 	return len(s)
 }
 
 func (s mockNodeStore) GetStoreDescriptor(id roachpb.StoreID) (*roachpb.StoreDescriptor, error) {
-	return nil, errorutil.NewStoreNotFoundError(id)
+	return nil, kvpb.NewStoreDescNotFoundError(id)
 }
 
 // TestOracle tests the Oracle exposed by this package.

diff --git a/pkg/gossip/gossip.go b/pkg/gossip/gossip.go
@@ -877,7 +877,7 @@ func (g *Gossip) getNodeDescriptor(
 		return nodeDescriptor, nil
 	}
 
-	return nil, errorutil.NewNodeNotFoundError(nodeID)
+	return nil, kvpb.NewNodeDescNotFoundError(nodeID)
 }
 
 // getNodeIDAddress looks up the address of the node by ID. The method accepts a

diff --git a/pkg/kv/kvclient/kvcoord/BUILD.bazel b/pkg/kv/kvclient/kvcoord/BUILD.bazel
@@ -206,7 +206,6 @@ go_test(
         "//pkg/util/caller",
         "//pkg/util/ctxgroup",
         "//pkg/util/encoding",
-        "//pkg/util/errorutil",
         "//pkg/util/grpcutil",
         "//pkg/util/hlc",
         "//pkg/util/leaktest",

diff --git a/pkg/kv/kvclient/kvcoord/replica_slice_test.go b/pkg/kv/kvclient/kvcoord/replica_slice_test.go
@@ -17,9 +17,9 @@ import (
 	"testing"
 	"time"
 
+	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/util"
-	"github.com/cockroachdb/cockroach/pkg/util/errorutil"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/shuffle"
@@ -39,7 +39,7 @@ func (ns *mockNodeStore) GetNodeDescriptor(nodeID roachpb.NodeID) (*roachpb.Node
 			return &nd, nil
 		}
 	}
-	return nil, errorutil.NewNodeNotFoundError(nodeID)
+	return nil, kvpb.NewNodeDescNotFoundError(nodeID)
 }
 
 // GetNodeDescriptorCount is part of the NodeDescStore interface.
@@ -51,7 +51,7 @@ func (ns *mockNodeStore) GetNodeDescriptorCount() int {
 func (ns *mockNodeStore) GetStoreDescriptor(
 	storeID roachpb.StoreID,
 ) (*roachpb.StoreDescriptor, error) {
-	return nil, errorutil.NewStoreNotFoundError(storeID)
+	return nil, kvpb.NewStoreDescNotFoundError(storeID)
 }
 
 func TestNewReplicaSlice(t *testing.T) {

diff --git a/pkg/kv/kvclient/kvtenant/BUILD.bazel b/pkg/kv/kvclient/kvtenant/BUILD.bazel
@@ -33,7 +33,6 @@ go_library(
         "//pkg/sql/pgwire/pgcode",
         "//pkg/sql/pgwire/pgerror",
         "//pkg/ts/tspb",
-        "//pkg/util/errorutil",
         "//pkg/util/grpcutil",
         "//pkg/util/hlc",
         "//pkg/util/log",

diff --git a/pkg/kv/kvclient/kvtenant/connector.go b/pkg/kv/kvclient/kvtenant/connector.go
@@ -39,7 +39,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/ts/tspb"
-	"github.com/cockroachdb/cockroach/pkg/util/errorutil"
 	"github.com/cockroachdb/cockroach/pkg/util/grpcutil"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -517,7 +516,7 @@ func (c *connector) GetNodeDescriptor(nodeID roachpb.NodeID) (*roachpb.NodeDescr
 	defer c.mu.RUnlock()
 	desc, ok := c.mu.nodeDescs[nodeID]
 	if !ok {
-		return nil, errorutil.NewNodeNotFoundError(nodeID)
+		return nil, kvpb.NewNodeDescNotFoundError(nodeID)
 	}
 	return desc, nil
 }
@@ -535,7 +534,7 @@ func (c *connector) GetStoreDescriptor(storeID roachpb.StoreID) (*roachpb.StoreD
 	defer c.mu.RUnlock()
 	desc, ok := c.mu.storeDescs[storeID]
 	if !ok {
-		return nil, errorutil.NewStoreNotFoundError(storeID)
+		return nil, kvpb.NewStoreDescNotFoundError(storeID)
 	}
 	return desc, nil
 }

diff --git a/pkg/kv/kvpb/BUILD.bazel b/pkg/kv/kvpb/BUILD.bazel
@@ -33,6 +33,7 @@ go_library(
         "//pkg/util/admission/admissionpb",
         "//pkg/util/buildutil",
         "//pkg/util/caller",
+        "//pkg/util/errorutil",
         "//pkg/util/hlc",
         "//pkg/util/humanizeutil",
         "//pkg/util/log",

diff --git a/pkg/kv/kvpb/errors.go b/pkg/kv/kvpb/errors.go
@@ -18,6 +18,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
 	"github.com/cockroachdb/cockroach/pkg/util/caller"
+	"github.com/cockroachdb/cockroach/pkg/util/errorutil"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/humanizeutil"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -1627,10 +1628,50 @@ func (e *MissingRecordError) SafeFormatError(p errors.Printer) (next error) {
 	return nil
 }
 
+// DescNotFoundError is reported when a descriptor is missing.
+type DescNotFoundError struct {
+	storeID roachpb.StoreID
+	nodeID  roachpb.NodeID
+	isStore bool
+}
+
+// NewStoreDescNotFoundError initializes a new DescNotFoundError for a missing
+// store descriptor.
+func NewStoreDescNotFoundError(storeID roachpb.StoreID) *DescNotFoundError {
+	return &DescNotFoundError{
+		storeID: storeID,
+		isStore: true,
+	}
+}
+
+// NewNodeDescNotFoundError initializes a new DescNotFoundError for a missing
+// node descriptor.
+func NewNodeDescNotFoundError(nodeID roachpb.NodeID) *DescNotFoundError {
+	return &DescNotFoundError{
+		nodeID:  nodeID,
+		isStore: false,
+	}
+}
+
+func (e *DescNotFoundError) Error() string {
+	return redact.Sprint(e).StripMarkers()
+}
+
+func (e *DescNotFoundError) SafeFormatError(p errors.Printer) (next error) {
+	if e.isStore {
+		p.Printf("store descriptor with store ID %d was not found", e.storeID)
+	} else {
+		p.Printf("node descriptor with node ID %d was not found", e.nodeID)
+	}
+	return nil
+}
+
 func init() {
 	errors.RegisterLeafDecoder(errors.GetTypeKey((*MissingRecordError)(nil)), func(_ context.Context, _ string, _ []string, _ proto.Message) error {
 		return &MissingRecordError{}
 	})
+	errorutilpath := reflect.TypeOf(errorutil.TempSentinel{}).PkgPath()
+	errors.RegisterTypeMigration(errorutilpath, "*errorutil.descriptorNotFound", &DescNotFoundError{})
 }
 
 var _ errors.SafeFormatter = &MissingRecordError{}

diff --git a/pkg/kv/kvserver/BUILD.bazel b/pkg/kv/kvserver/BUILD.bazel
@@ -478,7 +478,6 @@ go_test(
         "//pkg/util/circuit",
         "//pkg/util/ctxgroup",
         "//pkg/util/encoding",
-        "//pkg/util/errorutil",
         "//pkg/util/future",
         "//pkg/util/grunning",
         "//pkg/util/hlc",

diff --git a/pkg/kv/kvserver/store_test.go b/pkg/kv/kvserver/store_test.go
@@ -60,7 +60,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/testutils/gossiputil"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
-	"github.com/cockroachdb/cockroach/pkg/util/errorutil"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -155,7 +154,7 @@ func (m mockNodeStore) GetNodeDescriptorCount() int {
 }
 
 func (m mockNodeStore) GetStoreDescriptor(id roachpb.StoreID) (*roachpb.StoreDescriptor, error) {
-	return nil, errorutil.NewStoreNotFoundError(id)
+	return nil, kvpb.NewStoreDescNotFoundError(id)
 }
 
 type dummyFirstRangeProvider struct {

diff --git a/pkg/sql/crdb_internal.go b/pkg/sql/crdb_internal.go
@@ -4530,7 +4530,7 @@ CREATE TABLE crdb_internal.ranges_no_leases (
 				replicaLocalityDatum := tree.DNull
 				nodeDesc, err := p.ExecCfg().NodeDescs.GetNodeDescriptor(replica.NodeID)
 				if err != nil {
-					if !errorutil.IsDescriptorNotFoundError(err) {
+					if !errors.Is(err, &kvpb.DescNotFoundError{}) {
 						return nil, err
 					}
 				} else {

diff --git a/pkg/sql/crdb_internal_ranges_deprecated.go b/pkg/sql/crdb_internal_ranges_deprecated.go
@@ -15,6 +15,7 @@ import (
 	"sort"
 
 	"github.com/cockroachdb/cockroach/pkg/keys"
+	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
@@ -24,8 +25,8 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
-	"github.com/cockroachdb/cockroach/pkg/util/errorutil"
 	"github.com/cockroachdb/cockroach/pkg/util/stop"
+	"github.com/cockroachdb/errors"
 )
 
 // crdbInternalRangesViewDEPRECATED is the pre-v23.1
@@ -200,7 +201,7 @@ CREATE TABLE crdb_internal.ranges_no_leases (
 				replicaLocalityDatum := tree.DNull
 				nodeDesc, err := p.ExecCfg().NodeDescs.GetNodeDescriptor(replica.NodeID)
 				if err != nil {
-					if !errorutil.IsDescriptorNotFoundError(err) {
+					if !errors.Is(err, &kvpb.DescNotFoundError{}) {
 						return nil, err
 					}
 				} else {

diff --git a/pkg/util/errorutil/BUILD.bazel b/pkg/util/errorutil/BUILD.bazel
@@ -4,15 +4,14 @@ go_library(
     name = "errorutil",
     srcs = [
         "catch.go",
-        "descriptor.go",
         "error.go",
+        "sentinel.go",
         "tenant.go",
         "tenant_deprecated_wrapper.go",
     ],
     importpath = "github.com/cockroachdb/cockroach/pkg/util/errorutil",
     visibility = ["//visibility:public"],
     deps = [
-        "//pkg/roachpb",
         "//pkg/settings",
         "//pkg/util/errorutil/unimplemented",
         "//pkg/util/log/logcrash",

diff --git a/pkg/util/errorutil/descriptor.go b/pkg/util/errorutil/descriptor.go
diff --git a/pkg/util/errorutil/sentinel.go b/pkg/util/errorutil/sentinel.go
@@ -0,0 +1,15 @@
+// Copyright 2023 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+package errorutil
+
+// TempSentinel is a sentinel type that allows other packages to retrieve the
+// path to this package with reflect and PkgPath.
+type TempSentinel struct{}