Merge cockroachdb#92949 cockroachdb#94725 cockroachdb#94762

92949: ui: click to make graph legend sticky r=dhartunian a=dhartunian

Previously, hovering over a graph would make a legend appear but there was no way to persist the legend when the mouse moved away. This made carefully looking through denser graphs very challenging as it was tough to know which value corresponded to which series.

This commit introduces two changes to line graph behavior. First, clicking while hovering in a graph will stop the points from following the mouse, allowing you to cross-reference with the legend. Second, the legend will continue to persist even when you move the mouse away.

Epic: None

Resolves: cockroachdb#65031
Resolves: cockroachdb#65036

Release note (ui change): Graphs can be clicked on to toggle legend "stickiness" and make the points stop following the mouse. This makes it easier to read dense graphs with many series plotted together.

94725: storage: expose LazyValue for reverse scanning in pebbleMVCCScanner r=jbowens a=sumeerbhola

Informs cockroachdb/pebble#1170

Epic: CRDB-20378

94762: go.mod: bump Pebble to 10bdef44ad7c r=jbowens a=jbowens

```
10bdef44 Revert "db: require validity check before calling HasPointAndRange, RangeKeyChanged"
920d4db3 sstable: prevent duplicate put of valueBlockWriter to sync.Pool
3d9c6101 db: allow switching directions using NextPrefix
df2a3f09 db: factor out iterator repositioning to first/last key
1a5e2335 db: require validity check before calling HasPointAndRange, RangeKeyChanged
6cd8bb04 sstable: implement NextPrefix for the sstable iterators
811a8c0e replay: implement Stringer on workload collector's Cleaner
eb5e1039 replay: add workload pacing
36a9cc67 replay: stub out workload replay logic
```

Supersedes  cockroachdb#94712.

Epic: None
Release note: None

Co-authored-by: David Hartunian <[email protected]>
Co-authored-by: sumeerbhola <[email protected]>
Co-authored-by: Jackson Owens <[email protected]>

DEPS.bzl

@@ -1485,10 +1485,10 @@ def go_deps():
         patches = [
             "@com_github_cockroachdb_cockroach//build/patches:com_github_cockroachdb_pebble.patch",
         ],
-        sha256 = "1bc07fb5a406d3f408e7a58ce0186ff40fc0dc5d996773f465cd593d22e6ce51",
-        strip_prefix = "github.com/cockroachdb/[email protected]20221221181305-caecab07d786",
+        sha256 = "ff67b88929caeb1da76ab0c286a4837c51e86723d1d56bdb6150c329f436457f",
+        strip_prefix = "github.com/cockroachdb/[email protected]20230105164919-10bdef44ad7c",
         urls = [
-            "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20221221181305-caecab07d786.zip",
+            "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230105164919-10bdef44ad7c.zip",
         ],
     )
     go_repository(

build/bazelutil/distdir_files.bzl

@@ -193,7 +193,7 @@ DISTDIR_FILES = {
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/google-api-go-client/com_github_cockroachdb_google_api_go_client-v0.80.1-0.20221117193156-6a9f7150cb93.zip": "b3378c579f4f4340403038305907d672c86f615f8233118a8873ebe4229c4f39",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/gostdlib/com_github_cockroachdb_gostdlib-v1.19.0.zip": "c4d516bcfe8c07b6fc09b8a9a07a95065b36c2855627cb3514e40c98f872b69e",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/logtags/com_github_cockroachdb_logtags-v0.0.0-20211118104740-dabe8e521a4f.zip": "1972c3f171f118add3fd9e64bcea6cbb9959a3b7fa0ada308e8a7310813fea74",
-    "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20221221181305-caecab07d786.zip": "1bc07fb5a406d3f408e7a58ce0186ff40fc0dc5d996773f465cd593d22e6ce51",
+    "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/pebble/com_github_cockroachdb_pebble-v0.0.0-20230105164919-10bdef44ad7c.zip": "ff67b88929caeb1da76ab0c286a4837c51e86723d1d56bdb6150c329f436457f",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/redact/com_github_cockroachdb_redact-v1.1.3.zip": "7778b1e4485e4f17f35e5e592d87eb99c29e173ac9507801d000ad76dd0c261e",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/returncheck/com_github_cockroachdb_returncheck-v0.0.0-20200612231554-92cdbca611dd.zip": "ce92ba4352deec995b1f2eecf16eba7f5d51f5aa245a1c362dfe24c83d31f82b",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/sentry-go/com_github_cockroachdb_sentry_go-v0.6.1-cockroachdb.2.zip": "fbb2207d02aecfdd411b1357efe1192dbb827959e36b7cab7491731ac55935c9",

go.mod

@@ -111,7 +111,7 @@ require (
 	github.com/cockroachdb/go-test-teamcity v0.0.0-20191211140407-cff980ad0a55
 	github.com/cockroachdb/gostdlib v1.19.0
 	github.com/cockroachdb/logtags v0.0.0-20211118104740-dabe8e521a4f
-	github.com/cockroachdb/pebble v0.0.0-20221221181305-caecab07d786
+	github.com/cockroachdb/pebble v0.0.0-20230105164919-10bdef44ad7c
 	github.com/cockroachdb/redact v1.1.3
 	github.com/cockroachdb/returncheck v0.0.0-20200612231554-92cdbca611dd
 	github.com/cockroachdb/stress v0.0.0-20220803192808-1806698b1b7b

go.sum

@@ -474,8 +474,8 @@ github.com/cockroachdb/gostdlib v1.19.0/go.mod h1:+dqqpARXbE/gRDEhCak6dm0l14AaTy
 github.com/cockroachdb/logtags v0.0.0-20190617123548-eb05cc24525f/go.mod h1:i/u985jwjWRlyHXQbwatDASoW0RMlZ/3i9yJHE2xLkI=
 github.com/cockroachdb/logtags v0.0.0-20211118104740-dabe8e521a4f h1:6jduT9Hfc0njg5jJ1DdKCFPdMBrp/mdZfCpa5h+WM74=
 github.com/cockroachdb/logtags v0.0.0-20211118104740-dabe8e521a4f/go.mod h1:Vz9DsVWQQhf3vs21MhPMZpMGSht7O/2vFW2xusFUVOs=
-github.com/cockroachdb/pebble v0.0.0-20221221181305-caecab07d786 h1:KSCL38HF82L/pXHwetSLd8hYQTdyb84gn7dAoLxFO+s=
-github.com/cockroachdb/pebble v0.0.0-20221221181305-caecab07d786/go.mod h1:JsehdjcR1QgLZkqBeYrbVdE3cdxbdrycA/PN+Cg+RNw=
+github.com/cockroachdb/pebble v0.0.0-20230105164919-10bdef44ad7c h1:tqB+8UnqOLnMqgbKDro5IcpMEbs1Mqr8fMzCDKw2DAs=
+github.com/cockroachdb/pebble v0.0.0-20230105164919-10bdef44ad7c/go.mod h1:JsehdjcR1QgLZkqBeYrbVdE3cdxbdrycA/PN+Cg+RNw=
 github.com/cockroachdb/redact v1.1.3 h1:AKZds10rFSIj7qADf0g46UixK8NNLwWTNdCIGS5wfSQ=
 github.com/cockroachdb/redact v1.1.3/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZZ2lK+dpvRg=
 github.com/cockroachdb/returncheck v0.0.0-20200612231554-92cdbca611dd h1:KFOt5I9nEKZgCnOSmy8r4Oykh8BYQO8bFOTgHDS8YZA=

pkg/kv/kvserver/spanset/batch.go

@@ -229,6 +229,11 @@ func (i *MVCCIterator) IsPrefix() bool {
 	return i.i.IsPrefix()
 }
 
+// UnsafeLazyValue is part of the storage.MVCCIterator interface.
+func (i *MVCCIterator) UnsafeLazyValue() pebble.LazyValue {
+	return i.i.UnsafeLazyValue()
+}
+
 // EngineIterator wraps a storage.EngineIterator and ensures that it can
 // only be used to access spans in a SpanSet.
 type EngineIterator struct {

pkg/storage/engine.go

@@ -299,6 +299,14 @@ type MVCCIterator interface {
 	// IsPrefix returns true if the MVCCIterator is a prefix iterator, i.e.
 	// created with IterOptions.Prefix enabled.
 	IsPrefix() bool
+	// UnsafeLazyValue is only for use inside the storage package. It exposes
+	// the LazyValue at the current iterator position, and hence delays fetching
+	// the actual value. It is exposed for reverse scans that need to search for
+	// the most recent relevant version, and can't know whether the current
+	// value is that version, and need to step back to make that determination.
+	//
+	// REQUIRES: Valid() returns true.
+	UnsafeLazyValue() pebble.LazyValue
 }
 
 // EngineIterator is an iterator over key-value pairs where the key is

pkg/storage/intent_interleaving_iter.go

@@ -962,6 +962,13 @@ func (i *intentInterleavingIter) UnsafeValue() ([]byte, error) {
 	return i.iter.UnsafeValue()
 }
 
+func (i *intentInterleavingIter) UnsafeLazyValue() pebble.LazyValue {
+	if i.isCurAtIntentIter() {
+		return i.intentIter.UnsafeLazyValue()
+	}
+	return i.iter.UnsafeLazyValue()
+}
+
 func (i *intentInterleavingIter) MVCCValueLenAndIsTombstone() (int, bool, error) {
 	if i.isCurAtIntentIter() {
 		return 0, false, errors.Errorf("not at MVCC value")

pkg/storage/mvcc_history_metamorphic_iterator_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/storage"
 	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
 	"github.com/cockroachdb/cockroach/pkg/util/uuid"
+	"github.com/cockroachdb/pebble"
 	"github.com/stretchr/testify/require"
 )
 
@@ -348,6 +349,7 @@ func (m *metamorphicIterator) RangeKeyChanged() bool {
 }
 
 type metamorphicMVCCIterator struct {
+	// INVARIANT: metamorphicIterator.it implements MVCCIterator.
 	*metamorphicIterator
 }
 
@@ -363,6 +365,10 @@ func (m *metamorphicMVCCIterator) Prev() {
 	m.moveAround()
 }
 
+func (m *metamorphicMVCCIterator) UnsafeLazyValue() pebble.LazyValue {
+	return m.it.(storage.MVCCIterator).UnsafeLazyValue()
+}
+
 func (m *metamorphicMVCCIterator) SeekIntentGE(key roachpb.Key, txnUUID uuid.UUID) {
 	m.it.(storage.MVCCIterator).SeekIntentGE(key, txnUUID)
 	m.moveAround()

pkg/storage/pebble_iterator.go

@@ -554,6 +554,14 @@ func (p *pebbleIterator) UnsafeValue() ([]byte, error) {
 	return p.iter.ValueAndErr()
 }
 
+// UnsafeLazyValue implements the MVCCIterator interface.
+func (p *pebbleIterator) UnsafeLazyValue() pebble.LazyValue {
+	if ok := p.iter.Valid(); !ok {
+		panic(errors.AssertionFailedf("UnsafeLazyValue called on !Valid iterator"))
+	}
+	return p.iter.LazyValue()
+}
+
 // MVCCValueLenAndIsTombstone implements the MVCCIterator interface.
 func (p *pebbleIterator) MVCCValueLenAndIsTombstone() (int, bool, error) {
 	lv := p.iter.LazyValue()

pkg/storage/pebble_mvcc_scanner.go

@@ -376,13 +376,15 @@ type pebbleMVCCScanner struct {
 	failOnMoreRecent bool
 	keyBuf           []byte
 	savedBuf         []byte
+	lazyFetcherBuf   pebble.LazyFetcher
+	lazyValueBuf     []byte
 	// cur* variables store the "current" record we're pointing to. Updated in
 	// updateCurrent. Note that the timestamp can be clobbered in the case of
 	// adding an intent from the intent history but is otherwise meaningful.
 	curUnsafeKey   MVCCKey
 	curRawKey      []byte
 	curUnsafeValue MVCCValue
-	curRawValue    []byte
+	curRawValue    pebble.LazyValue
 	curRangeKeys   MVCCRangeKeyStack
 	savedRangeKeys MVCCRangeKeyStack
 	results        pebbleResults
@@ -692,10 +694,16 @@ func (p *pebbleMVCCScanner) getOne(ctx context.Context) (ok, added bool) {
 			return p.addSynthetic(ctx, p.curUnsafeKey.Key, rkv)
 		}
 
-		if extended, valid := p.tryDecodeCurrentValueSimple(); !valid {
+		// We are eagerly fetching and decoding the value, even though it may be
+		// too recent. With some care, this could be optimized to be lazy.
+		v, valid := p.getFromLazyValue()
+		if !valid {
+			return false, false
+		}
+		if extended, valid := p.tryDecodeCurrentValueSimple(v); !valid {
 			return false, false
 		} else if extended {
-			if !p.decodeCurrentValueExtended() {
+			if !p.decodeCurrentValueExtended(v) {
 				return false, false
 			}
 		}
@@ -1246,10 +1254,14 @@ func (p *pebbleMVCCScanner) seekVersion(
 		}
 		if p.curUnsafeKey.Timestamp.LessEq(seekTS) {
 			p.incrementItersBeforeSeek()
-			if extended, valid := p.tryDecodeCurrentValueSimple(); !valid {
+			v, valid := p.getFromLazyValue()
+			if !valid {
+				return false, false
+			}
+			if extended, valid := p.tryDecodeCurrentValueSimple(v); !valid {
 				return false, false
 			} else if extended {
-				if !p.decodeCurrentValueExtended() {
+				if !p.decodeCurrentValueExtended(v) {
 					return false, false
 				}
 			}
@@ -1284,10 +1296,14 @@ func (p *pebbleMVCCScanner) seekVersion(
 			p.setAdvanceKeyAtNewKey(origKey)
 			return true /* ok */, false
 		}
-		if extended, valid := p.tryDecodeCurrentValueSimple(); !valid {
+		v, valid := p.getFromLazyValue()
+		if !valid {
+			return false, false
+		}
+		if extended, valid := p.tryDecodeCurrentValueSimple(v); !valid {
 			return false, false
 		} else if extended {
-			if !p.decodeCurrentValueExtended() {
+			if !p.decodeCurrentValueExtended(v) {
 				return false, false
 			}
 		}
@@ -1447,11 +1463,7 @@ func (p *pebbleMVCCScanner) updateCurrent() bool {
 		p.err = errors.Wrap(err, "unable to decode MVCCKey")
 		return false
 	}
-	p.curRawValue, err = p.parent.UnsafeValue()
-	if err != nil {
-		p.err = err
-		return false
-	}
+	p.curRawValue = p.parent.UnsafeLazyValue()
 
 	// Reset decoded value to avoid bugs.
 	if util.RaceEnabled {
@@ -1461,12 +1473,28 @@ func (p *pebbleMVCCScanner) updateCurrent() bool {
 	return true
 }
 
+func (p *pebbleMVCCScanner) getFromLazyValue() (v []byte, valid bool) {
+	v, callerOwned, err := p.curRawValue.Value(p.lazyValueBuf)
+	if err != nil {
+		p.err = err
+		return nil, false
+	}
+	if callerOwned {
+		p.lazyValueBuf = v[:0]
+	}
+	return v, true
+}
+
 func (p *pebbleMVCCScanner) decodeCurrentMetadata() bool {
-	if len(p.curRawValue) == 0 {
+	val, valid := p.getFromLazyValue()
+	if !valid {
+		return false
+	}
+	if len(val) == 0 {
 		p.err = errors.Errorf("zero-length mvcc metadata")
 		return false
 	}
-	err := protoutil.Unmarshal(p.curRawValue, &p.meta)
+	err := protoutil.Unmarshal(val, &p.meta)
 	if err != nil {
 		p.err = errors.Wrap(err, "unable to decode MVCCMetadata")
 		return false
@@ -1475,15 +1503,15 @@ func (p *pebbleMVCCScanner) decodeCurrentMetadata() bool {
 }
 
 //gcassert:inline
-func (p *pebbleMVCCScanner) tryDecodeCurrentValueSimple() (extended, valid bool) {
+func (p *pebbleMVCCScanner) tryDecodeCurrentValueSimple(v []byte) (extended, valid bool) {
 	var simple bool
-	p.curUnsafeValue, simple, p.err = tryDecodeSimpleMVCCValue(p.curRawValue)
+	p.curUnsafeValue, simple, p.err = tryDecodeSimpleMVCCValue(v)
 	return !simple, p.err == nil
 }
 
 //gcassert:inline
-func (p *pebbleMVCCScanner) decodeCurrentValueExtended() bool {
-	p.curUnsafeValue, p.err = decodeExtendedMVCCValue(p.curRawValue)
+func (p *pebbleMVCCScanner) decodeCurrentValueExtended(v []byte) bool {
+	p.curUnsafeValue, p.err = decodeExtendedMVCCValue(v)
 	return p.err == nil
 }
 
@@ -1607,9 +1635,8 @@ func (p *pebbleMVCCScanner) iterPeekPrev() ([]byte, bool, bool) {
 		// curRawKey, curKey and curValue to point to this saved data. We use a
 		// single buffer for this purpose: savedBuf.
 		p.savedBuf = append(p.savedBuf[:0], p.curRawKey...)
-		p.savedBuf = append(p.savedBuf, p.curRawValue...)
+		p.curRawValue, p.savedBuf = p.curRawValue.Clone(p.savedBuf, &p.lazyFetcherBuf)
 		p.curRawKey = p.savedBuf[:len(p.curRawKey)]
-		p.curRawValue = p.savedBuf[len(p.curRawKey):]
 		// The raw key is always a prefix of the encoded MVCC key. Take advantage of this to
 		// sub-slice the raw key directly, instead of calling SplitMVCCKey.
 		p.curUnsafeKey.Key = p.curRawKey[:len(p.curUnsafeKey.Key)]
@@ -1690,7 +1717,11 @@ func (p *pebbleMVCCScanner) isKeyLockedByConflictingTxn(
 // addCurIntent adds the key-value pair that the scanner is currently
 // pointing to as an intent to the intents set.
 func (p *pebbleMVCCScanner) addCurIntent(ctx context.Context) bool {
-	return p.addRawIntent(ctx, p.curRawKey, p.curRawValue)
+	v, valid := p.getFromLazyValue()
+	if !valid {
+		return false
+	}
+	return p.addRawIntent(ctx, p.curRawKey, v)
 }
 
 // addKeyAndMetaAsIntent adds the key and transaction meta as an intent to

pkg/storage/verifying_iterator.go

@@ -13,6 +13,7 @@ package storage
 import (
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/util/uuid"
+	"github.com/cockroachdb/pebble"
 )
 
 // verifyingMVCCIterator is an MVCC iterator that wraps a pebbleIterator and
@@ -129,3 +130,8 @@ func (i *verifyingMVCCIterator) Valid() (bool, error) {
 func (i *verifyingMVCCIterator) HasPointAndRange() (bool, bool) {
 	return i.hasPoint, i.hasRange
 }
+
+// UnsafeLazyValue implements MVCCIterator.
+func (i *verifyingMVCCIterator) UnsafeLazyValue() pebble.LazyValue {
+	return pebble.LazyValue{ValueOrHandle: i.value}
+}

pkg/ui/workspaces/cluster-ui/src/graphs/visualization/visualizations.module.scss

@@ -61,7 +61,6 @@
 }
 
 .visualization-graph-sizing {
-  height: calc(30% - 200px);
   min-height: 300px;
 }
 

pkg/ui/workspaces/db-console/src/views/cluster/components/linegraph/linegraph.styl

@@ -26,6 +26,7 @@ $viz-sides = 62px
   height 100%
   .uplot
     display flex
+    flex-direction column
     > .u-legend
       display none
       position absolute
@@ -45,15 +46,13 @@ $viz-sides = 62px
     // stick the legend to the char's bottom.
     // Chart height is hardcoded to 300px (pkg/ui/src/views/cluster/util/graphs.ts)
     > .u-legend.u-legend--place-bottom
-      top 300px!important
-      left 0!important
+      position relative
+      top 0!important // Necessary because uPlot adds top/left on hover to make legend appear near mouse pointer
+      left 0!important // Necessary because uPlot adds top/left on hover to make legend appear near mouse pointer
       width auto
       margin-left -1px
       margin-right -1px
-      border-top none
-      border-radius 0px 0px 5px 5px
       padding-left 50px
-
       // the first line in the legend represents Y-axis and it occupies an entire line
       // to stand out from the rest of x-axis values
       > tr:first-child

pkg/ui/workspaces/db-console/src/views/cluster/util/graphs.ts

@@ -107,8 +107,18 @@ export function configureUPlotLineChart(
             legend.style.display = "block";
           };
 
+          // persistLegend determines if legend should continue showing even when mouse
+          // hovers away.
+          let persistLegend = false;
+
+          over.addEventListener("click", () => {
+            persistLegend = !persistLegend;
+          });
+
           over.onmouseleave = () => {
-            legend.style.display = "none";
+            if (!persistLegend) {
+              legend.style.display = "none";
+            }
           };
         },
         setCursor: (self: uPlot) => {
@@ -139,6 +149,9 @@ export function configureUPlotLineChart(
     //     key: "sync-everything",
     //   },
     // },
+    cursor: {
+      lock: true,
+    },
     legend: {
       show: true,