Skip to content

Commit 59e98df

Browse files
craig[bot]fqaziyuzefovichalyshanjahani-crldhartunian
committed
146547: sql/schemachanger: handle idempotent scenarios for schema_locked r=fqazi a=fqazi Previously, the declarative schema changer would automatically set/unset schema_locked on tables that are schema locked. This unfortunately also applied when operations were idempotent, which could lead to jobs that would needlessly toggle schema_locked. For example if a CREATE INDEX IF NOT EXISTS statement was issued, we would still toggle schema_locked. To address this, this patch only sets / unsets schema_locked if other elements are modified by the end of a build phase for a given operation. Fixes: #146541 Release note: None 147026: sql: remove old hyperloglog version r=yuzefovich a=yuzefovich This was only needed for compatibility with 24.3 nodes to ensure that table stats collection worked in mixed-version state. We no longer need compatibility with that version. Additionally remove no longer used sketch type field from the processor spec. We've bumped the min version as well as removed mixed-version 24.3 logic test config, so this should now be safe. Epic: None Release note: None 147434: ui: Fix collapsing behaviour on tables r=alyshanjahani-crl a=alyshanjahani-crl Previously, clicking the arrow icon to expand a region in the nodes table worked, but clicking the arrow to collapse did not. Instead, you had to click elsewhere on the row to collapse the region. This behaviour occurs since expandRowByClick is enabled on the table, and so once the row is expanded, clicking the arrow to collapse triggers the collapse and a subsequent expansion. Fixes: #146676 Release note: None 147442: jobs: add hints and details to job failure errors r=kyle-a-wong,msbutler a=dhartunian Resolves #137050 Release note: None 147462: db-console: change disk throughput graph titles to omit magnitude r=angles-n-daemons a=angles-n-daemons db-console: change disk throughput graph titles to omit magnitude In the metrics page, with the hardware graphs selected, you'll notice that the title of the disk throughput graphs include `MiB`. This is confusing, as the magnitude of the y-axis is programmatically determinted (it will be KiB, or GiB depending on how large the deltas are between time segments). To fix this, we strip the titles of their magnitude, going from MiB/s to simply Bytes/s, so that it's impossible for two prefixes show up in the same view. Fixes: #141003 Epic: none Release note (ui change): Changes the titles of the disk throughput graphs in the hardware metrics page to say only Bytes/s instead of MiB/s. Co-authored-by: Faizan Qazi <[email protected]> Co-authored-by: Yahor Yuzefovich <[email protected]> Co-authored-by: Alyshan Jahani <[email protected]> Co-authored-by: David Hartunian <[email protected]> Co-authored-by: Brian Dillmann <[email protected]>
6 parents d63e5f8 + 62ef26f + a88e686 + f725d4b + 20e9d57 + ac3c021 commit 59e98df

File tree

27 files changed

+116
-281
lines changed

27 files changed

+116
-281
lines changed

‎DEPS.bzl‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -783,16 +783,6 @@ def go_deps():
783783
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/axiomhq/hyperloglog/com_github_axiomhq_hyperloglog-v0.2.5.zip",
784784
],
785785
)
786-
go_repository(
787-
name = "com_github_axiomhq_hyperloglog_000",
788-
build_file_proto_mode = "disable_global",
789-
importpath = "github.com/axiomhq/hyperloglog/000",
790-
sha256 = "812834322ee2ca50dc36f91f9ac3f2cde4631af2f9c330b1271c78b46024a540",
791-
strip_prefix = "github.com/axiomhq/[email protected]",
792-
urls = [
793-
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/axiomhq/hyperloglog/com_github_axiomhq_hyperloglog-v0.0.0-20181223111420-4b99d0c2c99e.zip",
794-
],
795-
)
796786
go_repository(
797787
name = "com_github_aymanbagabas_go_osc52",
798788
build_file_proto_mode = "disable_global",

‎build/bazelutil/distdir_files.bzl‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,6 @@ DISTDIR_FILES = {
288288
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/aws/aws-sdk-go-v2/service/sts/com_github_aws_aws_sdk_go_v2_service_sts-v1.33.17.zip": "87aca25fafd483a1eac29c5baaab05ad485422a9aa1ccc5db0d39733c2d71cd2",
289289
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/aws/aws-sdk-go/com_github_aws_aws_sdk_go-v1.40.37.zip": "c0c481d28af88f621fb3fdeacc1e5d32f69a1bb83d0ee959f95ce89e4e2d0494",
290290
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/aws/smithy-go/com_github_aws_smithy_go-v1.22.3.zip": "572df48de9133d57f45909d3067b2053b97230268c2d28e4e44ea9644009ef11",
291-
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/axiomhq/hyperloglog/com_github_axiomhq_hyperloglog-v0.0.0-20181223111420-4b99d0c2c99e.zip": "812834322ee2ca50dc36f91f9ac3f2cde4631af2f9c330b1271c78b46024a540",
292291
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/axiomhq/hyperloglog/com_github_axiomhq_hyperloglog-v0.2.5.zip": "6125b12664bb5dd8614e82f0fe7528242dcb11649e1d7e051aabf3da471e14e1",
293292
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/aymanbagabas/go-osc52/com_github_aymanbagabas_go_osc52-v1.0.3.zip": "138e75a51599c2a8e4afe2bd6acdeaddbb73eb9ec796dfa2f577b16201660d9e",
294293
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/aymerick/douceur/com_github_aymerick_douceur-v0.2.0.zip": "dcbf69760cc1a8b32384495438e1086e4c3d669b2ebc0debd92e1865ffd6be60",

‎go.mod‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,6 @@ require (
122122
github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.35.1
123123
github.com/aws/smithy-go v1.22.3
124124
github.com/axiomhq/hyperloglog v0.2.5
125-
github.com/axiomhq/hyperloglog/000 v0.0.0-20181223111420-4b99d0c2c99e
126125
github.com/bazelbuild/rules_go v0.26.0
127126
github.com/biogo/store v0.0.0-20160505134755-913427a1d5e8
128127
github.com/blevesearch/snowballstem v0.9.0
@@ -507,10 +506,6 @@ replace github.com/gogo/protobuf => github.com/cockroachdb/gogoproto v1.3.3-0.20
507506

508507
replace storj.io/drpc => github.com/cockroachdb/drpc v0.0.0-20250507084558-a793c5c40d3d
509508

510-
// TODO(yuzefovich): remove this version once compatibility with 24.3 is no
511-
// longer needed.
512-
replace github.com/axiomhq/hyperloglog/000 => github.com/axiomhq/hyperloglog v0.0.0-20181223111420-4b99d0c2c99e
513-
514509
// Note: This forked dependency adds a commit that opens up some
515510
// private APIs to enable us to make some perf improvements to
516511
// histogram updates in particular.

‎go.sum‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -451,8 +451,6 @@ github.com/aws/aws-sdk-go-v2/service/sts v1.33.17/go.mod h1:cQnB8CUnxbMU82JvlqjK
451451
github.com/aws/smithy-go v1.13.5/go.mod h1:Tg+OJXh4MB2R/uN61Ko2f6hTZwB/ZYGOtib8J3gBHzA=
452452
github.com/aws/smithy-go v1.22.3 h1:Z//5NuZCSW6R4PhQ93hShNbyBbn8BWCmCVCt+Q8Io5k=
453453
github.com/aws/smithy-go v1.22.3/go.mod h1:t1ufH5HMublsJYulve2RKmHDC15xu1f26kHCp/HgceI=
454-
github.com/axiomhq/hyperloglog v0.0.0-20181223111420-4b99d0c2c99e h1:190ugM9MsyFauTkR/UqcHG/mn5nmFe6SvHJqEHIrtrA=
455-
github.com/axiomhq/hyperloglog v0.0.0-20181223111420-4b99d0c2c99e/go.mod h1:IOXAcuKIFq/mDyuQ4wyJuJ79XLMsmLM+5RdQ+vWrL7o=
456454
github.com/axiomhq/hyperloglog v0.2.5 h1:Hefy3i8nAs8zAI/tDp+wE7N+Ltr8JnwiW3875pvl0N8=
457455
github.com/axiomhq/hyperloglog v0.2.5/go.mod h1:DLUK9yIzpU5B6YFLjxTIcbHu1g4Y1WQb1m5RH3radaM=
458456
github.com/aymanbagabas/go-osc52 v1.0.3 h1:DTwqENW7X9arYimJrPeGZcV0ln14sGMt3pHZspWD+Mg=

‎pkg/jobs/jobs.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,16 @@ func (u Updater) failed(ctx context.Context, err error) error {
459459
jobErrMaxRuneCount = 1024
460460
jobErrTruncatedMarker = " -- TRUNCATED"
461461
)
462+
hints := errors.FlattenHints(err)
463+
details := errors.FlattenDetails(err)
462464
errStr := err.Error()
465+
if hints != "" {
466+
errStr += "\nHINT: " + hints
467+
}
468+
if details != "" {
469+
errStr += "\nDETAIL: " + details
470+
}
471+
463472
if len(errStr) > jobErrMaxRuneCount {
464473
errStr = util.TruncateString(errStr, jobErrMaxRuneCount) + jobErrTruncatedMarker
465474
}

‎pkg/sql/distsql_plan_stats.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,6 @@ func (dsp *DistSQLPlanner) createPartialStatsPlan(
410410

411411
sampledColumnIDs := make([]descpb.ColumnID, len(scan.catalogCols))
412412
spec := execinfrapb.SketchSpec{
413-
SketchType: execinfrapb.SketchType_HLL_PLUS_PLUS_V1,
414413
GenerateHistogram: reqStat.histogram,
415414
HistogramMaxBuckets: reqStat.histogramMaxBuckets,
416415
Columns: make([]uint32, len(reqStat.columns)),
@@ -656,7 +655,6 @@ func (dsp *DistSQLPlanner) createStatsPlan(
656655
sampledColumnIDs := make([]descpb.ColumnID, len(requestedCols))
657656
for _, s := range reqStats {
658657
spec := execinfrapb.SketchSpec{
659-
SketchType: execinfrapb.SketchType_HLL_PLUS_PLUS_V1,
660658
GenerateHistogram: s.histogram,
661659
HistogramMaxBuckets: s.histogramMaxBuckets,
662660
Columns: make([]uint32, len(s.columns)),

‎pkg/sql/execinfrapb/processors_table_stats.proto‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,9 @@ option go_package = "github.com/cockroachdb/cockroach/pkg/sql/execinfrapb";
1717
import "sql/catalog/descpb/structured.proto";
1818
import "gogoproto/gogo.proto";
1919

20-
// TODO(yuzefovich): this can be removed once compatibility with 24.3 is no
21-
// longer needed.
22-
enum SketchType {
23-
// This is the github.com/axiomhq/hyperloglog binary format (as of commit
24-
// 730eea1) for a sketch with precision 14. Values are encoded using their key
25-
// encoding, except integers which are encoded in 8 bytes (little-endian).
26-
HLL_PLUS_PLUS_V1 = 0;
27-
}
28-
2920
// SketchSpec contains the specification for a generated statistic.
3021
message SketchSpec {
31-
optional SketchType sketch_type = 1 [(gogoproto.nullable) = false];
22+
reserved 1;
3223

3324
// Each value is an index identifying a column in the input stream.
3425
// TODO(radu): currently only one column is supported.

‎pkg/sql/logictest/testdata/logic_test/alter_table‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ ALTER TABLE t ADD CONSTRAINT IF NOT EXISTS foo UNIQUE (b)
3636
statement ok
3737
ALTER TABLE t ADD CONSTRAINT IF NOT EXISTS foo UNIQUE (f)
3838

39-
skipif config local-schema-locked
4039
query TTTRT
4140
SELECT description, user_name, status, fraction_completed, error
4241
FROM crdb_internal.jobs
@@ -46,16 +45,6 @@ LIMIT 1
4645
----
4746
ALTER TABLE test.public.t ADD CONSTRAINT foo UNIQUE (b) root succeeded 1 ·
4847

49-
onlyif config local-schema-locked
50-
query TTTRT
51-
SELECT description, user_name, status, fraction_completed, error
52-
FROM crdb_internal.jobs
53-
WHERE job_type in ('NEW SCHEMA CHANGE', 'SCHEMA CHANGE')
54-
ORDER BY created DESC
55-
LIMIT 1
56-
----
57-
ALTER TABLE test.public.t ADD CONSTRAINT IF NOT EXISTS foo UNIQUE (f) root succeeded 1 ·
58-
5948
statement error pgcode 42P07 .* name \"foo\" already exists
6049
ALTER TABLE t ADD CONSTRAINT foo UNIQUE (b)
6150

‎pkg/sql/rowexec/BUILD.bazel‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@ go_library(
115115
"//pkg/util/tracing/tracingpb",
116116
"//pkg/util/vector",
117117
"@com_github_axiomhq_hyperloglog//:hyperloglog",
118-
"@com_github_axiomhq_hyperloglog_000//:000",
119118
"@com_github_cockroachdb_errors//:errors",
120119
"@com_github_cockroachdb_logtags//:logtags",
121120
"@com_github_cockroachdb_redact//:redact",

‎pkg/sql/rowexec/sample_aggregator.go‎

Lines changed: 11 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,12 @@ import (
1010
"math"
1111
"time"
1212

13-
hllNew "github.com/axiomhq/hyperloglog"
14-
hllOld "github.com/axiomhq/hyperloglog/000"
13+
"github.com/axiomhq/hyperloglog"
1514
"github.com/cockroachdb/cockroach/pkg/jobs"
1615
"github.com/cockroachdb/cockroach/pkg/server/telemetry"
1716
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
1817
"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
1918
"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
20-
"github.com/cockroachdb/cockroach/pkg/sql/execversion"
2119
"github.com/cockroachdb/cockroach/pkg/sql/isql"
2220
"github.com/cockroachdb/cockroach/pkg/sql/parser"
2321
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
@@ -103,7 +101,6 @@ func newSampleAggregator(
103101
return nil, errors.Errorf("histograms require one column")
104102
}
105103
}
106-
useNewHLL := execversion.FromContext(ctx) >= execversion.V25_1
107104

108105
// Limit the memory use by creating a child monitor with a hard limit.
109106
// The processor will disable histogram collection if this limit is not
@@ -144,14 +141,10 @@ func newSampleAggregator(
144141
for i := range spec.Sketches {
145142
s.sketches[i] = sketchInfo{
146143
spec: spec.Sketches[i],
144+
sketch: hyperloglog.New14(),
147145
numNulls: 0,
148146
numRows: 0,
149147
}
150-
if useNewHLL {
151-
s.sketches[i].sketchNew = hllNew.New14()
152-
} else {
153-
s.sketches[i].sketchOld = hllOld.New14()
154-
}
155148
if spec.Sketches[i].GenerateHistogram {
156149
sampleCols.Add(int(spec.Sketches[i].Columns[0]))
157150
}
@@ -173,14 +166,10 @@ func newSampleAggregator(
173166
s.invSr[col] = &sr
174167
s.invSketch[col] = &sketchInfo{
175168
spec: spec.InvertedSketches[i],
169+
sketch: hyperloglog.New14(),
176170
numNulls: 0,
177171
numRows: 0,
178172
}
179-
if useNewHLL {
180-
s.invSketch[col].sketchNew = hllNew.New14()
181-
} else {
182-
s.invSketch[col].sketchOld = hllOld.New14()
183-
}
184173
}
185174

186175
if err := s.Init(
@@ -372,6 +361,8 @@ func (s *sampleAggregator) mainLoop(
372361
func (s *sampleAggregator) processSketchRow(
373362
sketch *sketchInfo, row rowenc.EncDatumRow, da *tree.DatumAlloc,
374363
) error {
364+
var tmpSketch hyperloglog.Sketch
365+
375366
numRows, err := row[s.numRowsCol].GetInt()
376367
if err != nil {
377368
return err
@@ -398,22 +389,11 @@ func (s *sampleAggregator) processSketchRow(
398389
if d == tree.DNull {
399390
return errors.AssertionFailedf("NULL sketch data")
400391
}
401-
if sketch.sketchNew != nil {
402-
var tmpSketch hllNew.Sketch
403-
if err := tmpSketch.UnmarshalBinary([]byte(*d.(*tree.DBytes))); err != nil {
404-
return err
405-
}
406-
if err := sketch.sketchNew.Merge(&tmpSketch); err != nil {
407-
return errors.NewAssertionErrorWithWrappedErrf(err, "merging sketch data")
408-
}
409-
} else {
410-
var tmpSketch hllOld.Sketch
411-
if err := tmpSketch.UnmarshalBinary([]byte(*d.(*tree.DBytes))); err != nil {
412-
return err
413-
}
414-
if err := sketch.sketchOld.Merge(&tmpSketch); err != nil {
415-
return errors.NewAssertionErrorWithWrappedErrf(err, "merging sketch data")
416-
}
392+
if err := tmpSketch.UnmarshalBinary([]byte(*d.(*tree.DBytes))); err != nil {
393+
return err
394+
}
395+
if err := sketch.sketch.Merge(&tmpSketch); err != nil {
396+
return errors.NewAssertionErrorWithWrappedErrf(err, "merging sketch data")
417397
}
418398
return nil
419399
}
@@ -631,12 +611,7 @@ func (s *sampleAggregator) getAvgSize(si *sketchInfo) int64 {
631611
// getDistinctCount returns the number of distinct values in the given sketch,
632612
// optionally including null values.
633613
func (s *sampleAggregator) getDistinctCount(si *sketchInfo, includeNulls bool) int64 {
634-
var distinctCount int64
635-
if si.sketchNew != nil {
636-
distinctCount = int64(si.sketchNew.Estimate())
637-
} else {
638-
distinctCount = int64(si.sketchOld.Estimate())
639-
}
614+
distinctCount := int64(si.sketch.Estimate())
640615
if si.numNulls > 0 && !includeNulls {
641616
// Nulls are included in the estimate, so reduce the count by 1 if nulls are
642617
// not requested.

0 commit comments

Comments
 (0)