Merge #93127

93127: multitenant: handle missing NodeDescriptor in crdb_internal.ranges_no_leases r=arulajmani a=ecwall Fixes #92915 This change matches the previous behavior of using "" for locality if the NodeDescriptor is not found instead of returning an error when generating crdb_internal.ranges_no_leases. Release note: None Co-authored-by: Evan Wall <wall@cockroachlabs.com>
cockroachdb · Dec 8, 2022 · 51f951c · 51f951c
2 parents 223f580 + c9f9198
commit 51f951c
Show file tree

Hide file tree

Showing 12 changed files with 67 additions and 13 deletions.
diff --git a/pkg/ccl/kvccl/kvfollowerreadsccl/BUILD.bazel b/pkg/ccl/kvccl/kvfollowerreadsccl/BUILD.bazel
@@ -72,6 +72,7 @@ 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
@@ -39,6 +39,7 @@ 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"
@@ -496,11 +497,11 @@ func (s mockNodeStore) GetNodeDescriptor(id roachpb.NodeID) (*roachpb.NodeDescri
 			return desc, nil
 		}
 	}
-	return nil, errors.Errorf("unable to look up descriptor for n%d", id)
+	return nil, errorutil.NewNodeNotFoundError(id)
 }
 
 func (s mockNodeStore) GetStoreDescriptor(id roachpb.StoreID) (*roachpb.StoreDescriptor, error) {
-	return nil, errors.Errorf("unable to look up descriptor for store ID %d", id)
+	return nil, errorutil.NewStoreNotFoundError(id)
 }
 
 // TestOracle tests the Oracle exposed by this package.

diff --git a/pkg/ccl/kvccl/kvtenantccl/BUILD.bazel b/pkg/ccl/kvccl/kvtenantccl/BUILD.bazel
@@ -24,6 +24,7 @@ go_library(
         "//pkg/settings",
         "//pkg/spanconfig",
         "//pkg/util/contextutil",
+        "//pkg/util/errorutil",
         "//pkg/util/grpcutil",
         "//pkg/util/hlc",
         "//pkg/util/log",

diff --git a/pkg/ccl/kvccl/kvtenantccl/connector.go b/pkg/ccl/kvccl/kvtenantccl/connector.go
@@ -30,6 +30,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/settings"
 	"github.com/cockroachdb/cockroach/pkg/spanconfig"
 	"github.com/cockroachdb/cockroach/pkg/util/contextutil"
+	"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"
@@ -334,7 +335,7 @@ func (c *Connector) GetNodeDescriptor(nodeID roachpb.NodeID) (*roachpb.NodeDescr
 	defer c.mu.RUnlock()
 	desc, ok := c.mu.nodeDescs[nodeID]
 	if !ok {
-		return nil, errors.Errorf("unable to look up descriptor for n%d", nodeID)
+		return nil, errorutil.NewNodeNotFoundError(nodeID)
 	}
 	return desc, nil
 }
@@ -345,7 +346,7 @@ func (c *Connector) GetStoreDescriptor(storeID roachpb.StoreID) (*roachpb.StoreD
 	defer c.mu.RUnlock()
 	desc, ok := c.mu.storeDescs[storeID]
 	if !ok {
-		return nil, errors.Errorf("unable to look up descriptor for store ID %d", storeID)
+		return nil, errorutil.NewStoreNotFoundError(storeID)
 	}
 	return desc, nil
 }

diff --git a/pkg/gossip/gossip.go b/pkg/gossip/gossip.go
@@ -568,7 +568,7 @@ func (g *Gossip) GetStoreDescriptor(storeID roachpb.StoreID) (*roachpb.StoreDesc
 		desc := (*roachpb.StoreDescriptor)(value)
 		return desc, nil
 	}
-	return nil, errors.Errorf("unable to look up descriptor for store ID %d", storeID)
+	return nil, roachpb.NewStoreNotFoundError(storeID)
 }
 
 // LogStatus logs the current status of gossip such as the incoming and
@@ -997,7 +997,7 @@ func (g *Gossip) getNodeDescriptor(
 		return nodeDescriptor, nil
 	}
 
-	return nil, errors.Errorf("unable to look up descriptor for n%d", nodeID)
+	return nil, errorutil.NewNodeNotFoundError(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
@@ -192,6 +192,7 @@ go_test(
         "//pkg/util",
         "//pkg/util/caller",
         "//pkg/util/ctxgroup",
+        "//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
@@ -20,10 +20,10 @@ import (
 
 	"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"
-	"github.com/cockroachdb/errors"
 	"github.com/stretchr/testify/require"
 )
 
@@ -40,14 +40,14 @@ func (ns *mockNodeStore) GetNodeDescriptor(nodeID roachpb.NodeID) (*roachpb.Node
 			return &nd, nil
 		}
 	}
-	return nil, errors.Errorf("unable to look up descriptor for n%d", nodeID)
+	return nil, errorutil.NewNodeNotFoundError(nodeID)
 }
 
 // GetStoreDescriptor is part of the NodeDescStore interface.
 func (ns *mockNodeStore) GetStoreDescriptor(
 	storeID roachpb.StoreID,
 ) (*roachpb.StoreDescriptor, error) {
-	return nil, errors.Errorf("unable to look up descriptor for store ID %d", storeID)
+	return nil, errorutil.NewStoreNotFoundError(storeID)
 }
 
 func TestNewReplicaSlice(t *testing.T) {

diff --git a/pkg/kv/kvserver/BUILD.bazel b/pkg/kv/kvserver/BUILD.bazel
@@ -414,6 +414,7 @@ go_test(
         "//pkg/util/contextutil",
         "//pkg/util/ctxgroup",
         "//pkg/util/encoding",
+        "//pkg/util/errorutil",
         "//pkg/util/hlc",
         "//pkg/util/humanizeutil",
         "//pkg/util/leaktest",

diff --git a/pkg/kv/kvserver/store_test.go b/pkg/kv/kvserver/store_test.go
@@ -50,6 +50,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/testutils"
 	"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"
@@ -141,7 +142,7 @@ func (m mockNodeStore) GetNodeDescriptor(id roachpb.NodeID) (*roachpb.NodeDescri
 }
 
 func (m mockNodeStore) GetStoreDescriptor(id roachpb.StoreID) (*roachpb.StoreDescriptor, error) {
-	return nil, errors.Errorf("unable to look up descriptor for store ID %d", id)
+	return nil, errorutil.NewStoreNotFoundError(id)
 }
 
 type dummyFirstRangeProvider struct {

diff --git a/pkg/sql/crdb_internal.go b/pkg/sql/crdb_internal.go
@@ -3713,12 +3713,19 @@ CREATE TABLE crdb_internal.ranges_no_leases (
 
 			replicaLocalityArr := tree.NewDArray(types.String)
 			for _, replica := range votersAndNonVoters {
+				// The table should still be rendered even if node locality is unavailable,
+				// so use NULL if nodeDesc is not found.
+				// See https://github.com/cockroachdb/cockroach/issues/92915.
+				replicaLocalityDatum := tree.DNull
 				nodeDesc, err := p.ExecCfg().NodeDescs.GetNodeDescriptor(replica.NodeID)
 				if err != nil {
-					return nil, err
+					if !errorutil.IsDescriptorNotFoundError(err) {
+						return nil, err
+					}
+				} else {
+					replicaLocalityDatum = tree.NewDString(nodeDesc.Locality.String())
 				}
-				replicaLocality := tree.NewDString(nodeDesc.Locality.String())
-				if err := replicaLocalityArr.Append(replicaLocality); err != nil {
+				if err := replicaLocalityArr.Append(replicaLocalityDatum); err != nil {
 					return nil, err
 				}
 			}

diff --git a/pkg/util/errorutil/BUILD.bazel b/pkg/util/errorutil/BUILD.bazel
@@ -5,13 +5,15 @@ go_library(
     name = "errorutil",
     srcs = [
         "catch.go",
+        "descriptor.go",
         "error.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
@@ -0,0 +1,38 @@
+// Copyright 2019 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
+
+import (
+	"fmt"
+
+	"github.com/cockroachdb/cockroach/pkg/roachpb"
+	"github.com/cockroachdb/errors"
+)
+
+type descriptorNotFound struct {
+	msg string
+}
+
+func (e *descriptorNotFound) Error() string {
+	return e.msg
+}
+
+func IsDescriptorNotFoundError(err error) bool {
+	return errors.HasType(err, (*descriptorNotFound)(nil))
+}
+
+func NewNodeNotFoundError(nodeID roachpb.NodeID) error {
+	return &descriptorNotFound{fmt.Sprintf("unable to look up descriptor for n%d", nodeID)}
+}
+
+func NewStoreNotFoundError(storeID roachpb.StoreID) error {
+	return &descriptorNotFound{fmt.Sprintf("unable to look up descriptor for store ID %d", storeID)}
+}