Merge cockroachdb#107457 cockroachdb#107468

107457: ui: increase hot ranges page timeout r=zachlite a=zachlite

This commit increases the hot ranges request timeout to 30 minutes for both the initial fetch and the refresh.

Informs #cockroachdb#104269
Epic: none
Release note (bug fix): The timeout duration when loading the
Hot Ranges page has been increased to 30 minutes.

107468: kv: implement errors.Wrapper on sendError, deflake test r=knz a=nvanbenschoten

Fixes cockroachdb#107353.

This commit makes `sendError` implement the `errors.Wrapper` interface. This deflakes `TestDefaultConnectionDisruptionDoesNotInterfereWithSystemTraffic`, which was expecting a call to `require.ErrorIs` to find a `context.DeadlineExceeded` in an error chain that included a `sendError`.

Release note: None

Co-authored-by: zachlite <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>

Author: 3 people

pkg/kv/kvclient/kvcoord/dist_sender.go

@@ -1826,7 +1826,7 @@ func (ds *DistSender) sendPartialBatch(
 			// deduceRetryEarlyExitError() call below the loop is inhibited.
 			pErr = kvpb.NewError(err)
 			switch {
-			case errors.HasType(err, sendError{}):
+			case IsSendError(err):
 				// We've tried all the replicas without success. Either they're all
 				// down, or we're using an out-of-date range descriptor. Evict from the
 				// cache and try again with an updated descriptor. Re-sending the
@@ -2080,7 +2080,7 @@ func noMoreReplicasErr(ambiguousErr, lastAttemptErr error) error {
 	// one to return; we may want to remember the "best" error we've seen (for
 	// example, a NotLeaseHolderError conveys more information than a
 	// RangeNotFound).
-	return newSendError(fmt.Sprintf("sending to all replicas failed; last error: %s", lastAttemptErr))
+	return newSendError(errors.Wrap(lastAttemptErr, "sending to all replicas failed; last error"))
 }
 
 // defaultSendClosedTimestampPolicy is used when the closed timestamp policy
@@ -2463,7 +2463,7 @@ func (ds *DistSender) sendToReplicas(
 								log.VEventf(
 									ctx, 2, "transport incompatible with updated routing; bailing early",
 								)
-								return nil, newSendError(fmt.Sprintf("leaseholder not found in transport; last error: %s", tErr.Error()))
+								return nil, newSendError(errors.Wrap(tErr, "leaseholder not found in transport; last error"))
 							}
 						}
 					}
@@ -2705,28 +2705,32 @@ func skipStaleReplicas(
 // TODO(andrei): clean up this stuff and tighten the meaning of the different
 // errors.
 type sendError struct {
-	message string
+	cause error
 }
 
-// newSendError creates a sendError.
-func newSendError(msg string) error {
-	return sendError{message: msg}
+// newSendError creates a sendError that wraps the given error.
+func newSendError(err error) error {
+	return &sendError{cause: err}
 }
 
 // TestNewSendError creates a new sendError for the purpose of unit tests
 func TestNewSendError(msg string) error {
-	return newSendError(msg)
+	return newSendError(errors.NewWithDepthf(1, "%s", msg))
 }
 
-// SendErrorString is the prefix for all sendErrors, exported in order to
-// perform cross-node error-checks.
-const SendErrorString = "failed to send RPC"
-
-func (s sendError) Error() string {
-	return SendErrorString + ": " + s.message
+// Error implements error.
+func (s *sendError) Error() string {
+	return fmt.Sprintf("failed to send RPC: %s", s.cause)
 }
 
+// Cause implements errors.Causer.
+// NB: this is an obsolete method, use Unwrap() instead.
+func (s *sendError) Cause() error { return s.cause }
+
+// Unwrap implements errors.Wrapper.
+func (s *sendError) Unwrap() error { return s.cause }
+
 // IsSendError returns true if err is a sendError.
 func IsSendError(err error) bool {
-	return errors.HasType(err, sendError{})
+	return errors.HasType(err, &sendError{})
 }

pkg/kv/kvclient/kvcoord/dist_sender_rangefeed.go

@@ -736,7 +736,7 @@ func (ds *DistSender) singleRangeFeed(
 	for {
 		stuckWatcher.stop() // if timer is running from previous iteration, stop it now
 		if transport.IsExhausted() {
-			return args.Timestamp, newSendError("sending to all replicas failed")
+			return args.Timestamp, newSendError(errors.New("sending to all replicas failed"))
 		}
 		maybeCleanupStream()
 

pkg/kv/kvclient/kvcoord/dist_sender_test.go

@@ -3662,7 +3662,7 @@ func TestMultipleErrorsMerged(t *testing.T) {
 	abortErr := kvpb.NewTransactionAbortedError(kvpb.ABORT_REASON_ABORTED_RECORD_FOUND)
 	conditionFailedErr := &kvpb.ConditionFailedError{}
 	writeIntentErr := &kvpb.WriteIntentError{}
-	sendErr := sendError{}
+	sendErr := &sendError{}
 	ambiguousErr := &kvpb.AmbiguousResultError{}
 	randomErr := &kvpb.IntegerOverflowError{}
 
@@ -4439,7 +4439,7 @@ func TestEvictionTokenCoalesce(t *testing.T) {
 			// Return a sendError so DistSender retries the first range lookup in the
 			// user key-space for both batches.
 			if previouslyWaited := waitForInitialPuts(); !previouslyWaited {
-				return nil, newSendError("boom")
+				return nil, TestNewSendError("boom")
 			}
 			return br, nil
 		}
@@ -4744,7 +4744,7 @@ func TestRequestSubdivisionAfterDescriptorChangeWithUnavailableReplicasTerminate
 	transportFn := func(_ context.Context, ba *kvpb.BatchRequest) (*kvpb.BatchResponse, error) {
 		atomic.AddInt32(&numAttempts, 1)
 		require.Equal(t, 1, len(ba.Requests))
-		return nil, newSendError("boom")
+		return nil, TestNewSendError("boom")
 	}
 	rpcRetryOptions := &retry.Options{
 		MaxRetries: 5, // maxAttempts = 6
@@ -5169,7 +5169,7 @@ func TestSendToReplicasSkipsStaleReplicas(t *testing.T) {
 				get.Key = roachpb.Key("a")
 				ba.Add(get)
 				_, err = ds.sendToReplicas(ctx, ba, tok, false /* withCommit */)
-				require.IsType(t, sendError{}, err)
+				require.IsType(t, &sendError{}, err)
 				require.Regexp(t, "NotLeaseHolderError", err)
 				cached := rc.GetCached(ctx, tc.initialDesc.StartKey, false /* inverted */)
 				require.NotNil(t, cached)

pkg/kv/kvclient/kvcoord/replica_slice.go

@@ -12,13 +12,13 @@ package kvcoord
 
 import (
 	"context"
-	"fmt"
 	"sort"
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/shuffle"
+	"github.com/cockroachdb/errors"
 )
 
 // ReplicaInfo extends the Replica structure with the associated node
@@ -134,7 +134,7 @@ func NewReplicaSlice(
 	}
 	if len(rs) == 0 {
 		return nil, newSendError(
-			fmt.Sprintf("no replica node information available via gossip for r%d", desc.RangeID))
+			errors.Errorf("no replica node information available via gossip for r%d", desc.RangeID))
 	}
 	return rs, nil
 }

pkg/ui/workspaces/db-console/src/redux/apiReducers.ts

@@ -126,6 +126,9 @@ export const hotRangesReducerObj = new PaginatedCachedDataReducer(
   api.getHotRanges,
   "hotRanges",
   hotRangesRequestToID,
+  1000 /* page limit */,
+  null /* invalidation period */,
+  moment.duration(30, "minutes"),
 );
 
 export const refreshDatabaseDetails = databaseDetailsReducerObj.refresh;

pkg/ui/workspaces/db-console/src/views/reports/containers/hotranges/index.tsx

@@ -38,7 +38,7 @@ const HotRanges = (props: HotRangesProps) => {
       page_size: pageSize,
       page_token: pageToken,
     });
-    getHotRanges(request).then(response => {
+    getHotRanges(request, moment.duration(30, "minutes")).then(response => {
       if (response.ranges.length == 0) {
         return;
       }