Skip to content

Commit 82eca8e

Browse files
craig[bot]williamchoe3dhartunianandyyang890yuzefovich
committed
155368: roachprod: implement GetVMSpecs for IBM r=DarrylWong,golgeek a=williamchoe3 Informs #146202 Implementation for previously unimplemented interface method GetVMSpecs() for IBM. This will allow for cluster information to be fetched in roachtest via FetchVMSpecs * API: https://cloud.ibm.com/apidocs/resource-controller/resource-controller?code=go#get-resource-instance Also added comments to help contextualize some parts that I thought would be helpful while doing some debug. See comment for e.g. vm specs 156481: telemetryccl: unskip and simplify TestTelemetry r=kyle-a-wong a=dhartunian Previously, we would often get test flakes for TestTelemetry because it exercised many different multi-region features for the sole purpose of checking whether telemetry counters would get incremented. This is not very useful because the logic for the different counters is mostly identical and runs through the same functions in multiregion/telemetry.go. As long as these integration tests verify that those functions get called, we don't need an exhaustive set of cases. Resolves: #152123 Epic: None Release note: none 156985: roachtest: deflake cdc/ledger r=log-head,asg0451 a=andyyang890 This patch deflakes `cdc/ledger` by increasing the target steady latency from 1 minute to 3 minutes. It also eliminates a weird quirk where the latency verifier expects the latency to be less than half of the target latency before it'll be considered steady, which led to weird logs and added unnecessary complexity. Fixes #151381 Release note: None 157018: sql: don't store plan gist as FastValue in context r=yuzefovich a=yuzefovich Given that in 67e45fd we introduced a different way to include plan gists into sentry reports, I don't think we need to include the plan gists into the context anymore, so we can free up the FastValue context slot for a better use. Epic: None Release note: None Co-authored-by: William Choe <[email protected]> Co-authored-by: David Hartunian <[email protected]> Co-authored-by: Andy Yang <[email protected]> Co-authored-by: Yahor Yuzefovich <[email protected]>
5 parents a627560 + 3c0cfae + 3777f5c + 08ce8f9 + 558347a commit 82eca8e

File tree

13 files changed

+147
-265
lines changed

13 files changed

+147
-265
lines changed

‎pkg/ccl/telemetryccl/telemetry_test.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@ func TestTelemetry(t *testing.T) {
2222
skip.UnderRace(t, "takes >1min under race")
2323
skip.UnderDeadlock(t, "takes >1min under deadlock")
2424

25-
skip.WithIssue(t, 152123)
26-
2725
sqltestutils.TelemetryTest(
2826
t,
2927
[]base.TestServerArgs{

‎pkg/ccl/telemetryccl/testdata/telemetry/multiregion_row‎

Lines changed: 23 additions & 127 deletions
Original file line numberDiff line numberDiff line change
@@ -12,165 +12,61 @@ USE d;
1212
ALTER DATABASE d ADD REGION "ap-southeast-2"
1313
----
1414

15+
#####################################################################
16+
# CREATE TABLE: Test each locality type once
17+
#####################################################################
18+
1519
feature-usage
16-
CREATE TABLE t4 () WITH (schema_locked=false) LOCALITY GLOBAL
20+
CREATE TABLE t_global () WITH (schema_locked=false) LOCALITY GLOBAL
1721
----
1822
sql.multiregion.create_table.locality.global
1923

2024
feature-usage
21-
CREATE TABLE t5 () WITH (schema_locked=false) LOCALITY REGIONAL BY ROW
25+
CREATE TABLE t_row () WITH (schema_locked=false) LOCALITY REGIONAL BY ROW
2226
----
2327
sql.multiregion.create_table.locality.regional_by_row
2428

2529
feature-usage
26-
CREATE TABLE t6 (cr crdb_internal_region) WITH (schema_locked=false) LOCALITY REGIONAL BY ROW AS cr
30+
CREATE TABLE t_row_as (cr crdb_internal_region) WITH (schema_locked=false) LOCALITY REGIONAL BY ROW AS cr
2731
----
2832
sql.multiregion.create_table.locality.regional_by_row_as
2933

30-
#
31-
# GLOBAL -> the others
32-
#
34+
#####################################################################
35+
# ALTER TABLE: Test representative transitions (not exhaustive)
36+
#####################################################################
3337

38+
# Test GLOBAL -> REGIONAL BY ROW transition
3439
feature-usage
35-
ALTER TABLE t4 SET LOCALITY REGIONAL BY ROW
40+
ALTER TABLE t_global SET LOCALITY REGIONAL BY ROW
3641
----
3742
sql.multiregion.alter_table.locality.from.global.to.regional_by_row
3843

3944
exec
40-
ALTER TABLE t4 SET LOCALITY GLOBAL
41-
----
42-
43-
feature-usage
44-
ALTER TABLE t4 SET LOCALITY GLOBAL
45-
----
46-
sql.multiregion.alter_table.locality.from.global.to.global
47-
48-
exec
49-
ALTER TABLE t4 SET LOCALITY GLOBAL
50-
----
51-
52-
feature-usage
53-
ALTER TABLE t4 SET LOCALITY REGIONAL BY TABLE IN "ap-southeast-2"
54-
----
55-
sql.multiregion.alter_table.locality.from.global.to.regional_by_table_in
56-
57-
exec
58-
ALTER TABLE t4 SET LOCALITY GLOBAL;
59-
ALTER TABLE t4 ADD COLUMN cr crdb_internal_region NOT NULL
60-
----
61-
62-
feature-usage
63-
ALTER TABLE t4 SET LOCALITY REGIONAL BY ROW AS "cr"
64-
----
65-
sql.multiregion.alter_table.locality.from.global.to.regional_by_row_as
66-
67-
exec
68-
ALTER TABLE t4 SET LOCALITY GLOBAL
45+
ALTER TABLE t_global SET LOCALITY GLOBAL
6946
----
7047

48+
# Test REGIONAL BY ROW -> GLOBAL transition
7149
feature-usage
72-
ALTER TABLE t4 SET LOCALITY REGIONAL BY TABLE
73-
----
74-
sql.multiregion.alter_table.locality.from.global.to.regional_by_table
75-
76-
exec
77-
ALTER TABLE t4 SET LOCALITY GLOBAL
78-
----
79-
80-
#
81-
# REGIONAL BY ROW -> the others
82-
#
83-
84-
feature-usage
85-
ALTER TABLE t5 SET LOCALITY REGIONAL BY ROW
86-
----
87-
sql.multiregion.alter_table.locality.from.regional_by_row.to.regional_by_row
88-
89-
exec
90-
ALTER TABLE t5 SET LOCALITY REGIONAL BY ROW
91-
----
92-
93-
feature-usage
94-
ALTER TABLE t5 SET LOCALITY GLOBAL
50+
ALTER TABLE t_row SET LOCALITY GLOBAL
9551
----
9652
sql.multiregion.alter_table.locality.from.regional_by_row.to.global
9753

9854
exec
99-
ALTER TABLE t5 SET LOCALITY REGIONAL BY ROW
100-
----
101-
102-
feature-usage
103-
ALTER TABLE t5 SET LOCALITY REGIONAL BY TABLE IN "ap-southeast-2"
104-
----
105-
sql.multiregion.alter_table.locality.from.regional_by_row.to.regional_by_table_in
106-
107-
exec
108-
ALTER TABLE t5 SET LOCALITY REGIONAL BY ROW;
109-
----
110-
111-
exec
112-
ALTER TABLE t5 ADD COLUMN cr crdb_internal_region NOT NULL
113-
----
114-
115-
feature-usage
116-
ALTER TABLE t5 SET LOCALITY REGIONAL BY ROW AS "cr"
117-
----
118-
sql.multiregion.alter_table.locality.from.regional_by_row.to.regional_by_row_as
119-
120-
exec
121-
ALTER TABLE t5 SET LOCALITY REGIONAL BY ROW
55+
ALTER TABLE t_row SET LOCALITY REGIONAL BY ROW
12256
----
12357

58+
# Test REGIONAL BY ROW AS -> REGIONAL BY TABLE transition
12459
feature-usage
125-
ALTER TABLE t5 SET LOCALITY REGIONAL BY TABLE
60+
ALTER TABLE t_row_as SET LOCALITY REGIONAL BY TABLE
12661
----
127-
sql.multiregion.alter_table.locality.from.regional_by_row.to.regional_by_table
128-
129-
exec
130-
ALTER TABLE t5 SET LOCALITY REGIONAL BY ROW
131-
----
132-
133-
#
134-
# REGIONAL BY TABLE -> the others
135-
#
136-
137-
feature-usage
138-
ALTER TABLE t6 SET LOCALITY REGIONAL BY ROW
139-
----
140-
sql.multiregion.alter_table.locality.from.regional_by_row_as.to.regional_by_row
141-
142-
exec
143-
ALTER TABLE t6 SET LOCALITY REGIONAL BY ROW AS "cr"
144-
----
145-
146-
feature-usage
147-
ALTER TABLE t6 SET LOCALITY GLOBAL
148-
----
149-
sql.multiregion.alter_table.locality.from.regional_by_row_as.to.global
150-
151-
exec
152-
ALTER TABLE t6 SET LOCALITY REGIONAL BY ROW AS "cr"
153-
----
154-
155-
feature-usage
156-
ALTER TABLE t6 SET LOCALITY REGIONAL BY TABLE IN "ap-southeast-2"
157-
----
158-
sql.multiregion.alter_table.locality.from.regional_by_row_as.to.regional_by_table_in
159-
160-
exec
161-
ALTER TABLE t6 SET LOCALITY REGIONAL BY ROW AS "cr"
162-
----
163-
164-
feature-usage
165-
ALTER TABLE t6 SET LOCALITY REGIONAL BY ROW AS "cr"
166-
----
167-
sql.multiregion.alter_table.locality.from.regional_by_row_as.to.regional_by_row_as
62+
sql.multiregion.alter_table.locality.from.regional_by_row_as.to.regional_by_table
16863

16964
exec
170-
ALTER TABLE t6 SET LOCALITY REGIONAL BY ROW AS "cr"
65+
ALTER TABLE t_row_as SET LOCALITY REGIONAL BY ROW AS "cr"
17166
----
17267

68+
# Test a self-transition case
17369
feature-usage
174-
ALTER TABLE t6 SET LOCALITY REGIONAL BY TABLE
70+
ALTER TABLE t_row SET LOCALITY REGIONAL BY ROW
17571
----
176-
sql.multiregion.alter_table.locality.from.regional_by_row_as.to.regional_by_table
72+
sql.multiregion.alter_table.locality.from.regional_by_row.to.regional_by_row

‎pkg/ccl/telemetryccl/testdata/telemetry/multiregion_table‎

Lines changed: 20 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -13,122 +13,61 @@ ALTER DATABASE d ADD REGION "ap-southeast-2"
1313
----
1414

1515
#####################################################################
16-
# CREATE TABLE: Test initial table creation with different localities
16+
# CREATE TABLE: Test each locality type once
1717
#####################################################################
1818

1919
feature-usage
20-
CREATE TABLE t0 ()
20+
CREATE TABLE t_unspecified ()
2121
----
2222
sql.multiregion.create_table.locality.unspecified
2323

2424
feature-usage
25-
CREATE TABLE t1 () LOCALITY REGIONAL BY TABLE
25+
CREATE TABLE t_regional () LOCALITY REGIONAL BY TABLE
2626
----
2727
sql.multiregion.create_table.locality.regional_by_table
2828

2929
feature-usage
30-
CREATE TABLE t2 () LOCALITY REGIONAL BY TABLE IN "ap-southeast-2"
30+
CREATE TABLE t_regional_in () LOCALITY REGIONAL BY TABLE IN "ap-southeast-2"
3131
----
3232
sql.multiregion.create_table.locality.regional_by_table_in
3333

3434
feature-usage
35-
CREATE TABLE t3 () LOCALITY GLOBAL
35+
CREATE TABLE t_global () LOCALITY GLOBAL
3636
----
3737
sql.multiregion.create_table.locality.global
3838

3939
#####################################################################
40-
# ALTER TABLE from REGIONAL BY TABLE
40+
# ALTER TABLE: Test representative transitions (not exhaustive)
4141
#####################################################################
4242

4343
exec
44-
CREATE TABLE t1_to_row () WITH (schema_locked=false) LOCALITY REGIONAL BY TABLE;
45-
CREATE TABLE t1_to_global () WITH (schema_locked=false) LOCALITY REGIONAL BY TABLE;
46-
CREATE TABLE t1_to_table_in () WITH (schema_locked=false) LOCALITY REGIONAL BY TABLE;
47-
CREATE TABLE t1_to_row_as () WITH (schema_locked=false) LOCALITY REGIONAL BY TABLE;
48-
ALTER TABLE t1_to_row_as ADD COLUMN cr crdb_internal_region NOT NULL;
44+
CREATE TABLE t_alter1 () WITH (schema_locked=false) LOCALITY REGIONAL BY TABLE;
45+
CREATE TABLE t_alter2 () WITH (schema_locked=false) LOCALITY REGIONAL BY TABLE IN "ap-southeast-2";
46+
CREATE TABLE t_alter3 () WITH (schema_locked=false) LOCALITY GLOBAL;
47+
CREATE TABLE t_alter4 () WITH (schema_locked=false) LOCALITY REGIONAL BY TABLE;
48+
ALTER TABLE t_alter4 ADD COLUMN cr crdb_internal_region NOT NULL;
4949
----
5050

51+
# Test REGIONAL BY TABLE -> REGIONAL BY ROW
5152
feature-usage
52-
ALTER TABLE t1_to_row SET LOCALITY REGIONAL BY ROW
53+
ALTER TABLE t_alter1 SET LOCALITY REGIONAL BY ROW
5354
----
5455
sql.multiregion.alter_table.locality.from.regional_by_table.to.regional_by_row
5556

57+
# Test REGIONAL BY TABLE IN -> GLOBAL
5658
feature-usage
57-
ALTER TABLE t1_to_global SET LOCALITY GLOBAL
58-
----
59-
sql.multiregion.alter_table.locality.from.regional_by_table.to.global
60-
61-
feature-usage
62-
ALTER TABLE t1_to_table_in SET LOCALITY REGIONAL BY TABLE IN "ap-southeast-2"
63-
----
64-
sql.multiregion.alter_table.locality.from.regional_by_table.to.regional_by_table_in
65-
66-
feature-usage
67-
ALTER TABLE t1_to_row_as SET LOCALITY REGIONAL BY ROW AS "cr"
68-
----
69-
sql.multiregion.alter_table.locality.from.regional_by_table.to.regional_by_row_as
70-
71-
#####################################################################
72-
# ALTER TABLE from REGIONAL BY TABLE IN
73-
#####################################################################
74-
75-
exec
76-
CREATE TABLE t2_to_row () WITH (schema_locked = false) LOCALITY REGIONAL BY TABLE IN "ap-southeast-2";
77-
CREATE TABLE t2_to_global () WITH (schema_locked = false) LOCALITY REGIONAL BY TABLE IN "ap-southeast-2";
78-
CREATE TABLE t2_to_table () WITH (schema_locked = false) LOCALITY REGIONAL BY TABLE IN "ap-southeast-2";
79-
CREATE TABLE t2_to_row_as () WITH (schema_locked = false) LOCALITY REGIONAL BY TABLE IN "ap-southeast-2";
80-
ALTER TABLE t2_to_row_as ADD COLUMN cr crdb_internal_region NOT NULL;
81-
----
82-
83-
feature-usage
84-
ALTER TABLE t2_to_row SET LOCALITY REGIONAL BY ROW
85-
----
86-
sql.multiregion.alter_table.locality.from.regional_by_table_in.to.regional_by_row
87-
88-
feature-usage
89-
ALTER TABLE t2_to_global SET LOCALITY GLOBAL
59+
ALTER TABLE t_alter2 SET LOCALITY GLOBAL
9060
----
9161
sql.multiregion.alter_table.locality.from.regional_by_table_in.to.global
9262

63+
# Test GLOBAL -> REGIONAL BY TABLE IN
9364
feature-usage
94-
ALTER TABLE t2_to_table SET LOCALITY REGIONAL BY TABLE
95-
----
96-
sql.multiregion.alter_table.locality.from.regional_by_table_in.to.regional_by_table
97-
98-
feature-usage
99-
ALTER TABLE t2_to_row_as SET LOCALITY REGIONAL BY ROW AS "cr"
100-
----
101-
sql.multiregion.alter_table.locality.from.regional_by_table_in.to.regional_by_row_as
102-
103-
104-
#####################################################################
105-
# ALTER TABLE from GLOBAL
106-
#####################################################################exec
107-
108-
exec
109-
CREATE TABLE t3_to_row () WITH (schema_locked = false) LOCALITY GLOBAL;
110-
CREATE TABLE t3_to_table () WITH (schema_locked = false) LOCALITY GLOBAL;
111-
CREATE TABLE t3_to_table_in () WITH (schema_locked = false) LOCALITY GLOBAL;
112-
CREATE TABLE t3_to_row_as () WITH (schema_locked = false) LOCALITY GLOBAL;
113-
ALTER TABLE t3_to_row_as ADD COLUMN cr crdb_internal_region NOT NULL;
114-
----
115-
116-
feature-usage
117-
ALTER TABLE t3_to_row SET LOCALITY REGIONAL BY ROW
118-
----
119-
sql.multiregion.alter_table.locality.from.global.to.regional_by_row
120-
121-
feature-usage
122-
ALTER TABLE t3_to_table SET LOCALITY REGIONAL BY TABLE
123-
----
124-
sql.multiregion.alter_table.locality.from.global.to.regional_by_table
125-
126-
feature-usage
127-
ALTER TABLE t3_to_table_in SET LOCALITY REGIONAL BY TABLE IN "ap-southeast-2"
65+
ALTER TABLE t_alter3 SET LOCALITY REGIONAL BY TABLE IN "ap-southeast-2"
12866
----
12967
sql.multiregion.alter_table.locality.from.global.to.regional_by_table_in
13068

69+
# Test REGIONAL BY TABLE -> REGIONAL BY ROW AS
13170
feature-usage
132-
ALTER TABLE t3_to_row_as SET LOCALITY REGIONAL BY ROW AS "cr"
71+
ALTER TABLE t_alter4 SET LOCALITY REGIONAL BY ROW AS "cr"
13372
----
134-
sql.multiregion.alter_table.locality.from.global.to.regional_by_row_as
73+
sql.multiregion.alter_table.locality.from.regional_by_table.to.regional_by_row_as

‎pkg/cmd/roachtest/tests/cdc.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2574,7 +2574,7 @@ CONFIGURE ZONE USING
25742574
})
25752575
ct.runFeedLatencyVerifier(feed, latencyTargets{
25762576
initialScanLatency: 10 * time.Minute,
2577-
steadyLatency: time.Minute,
2577+
steadyLatency: 3 * time.Minute,
25782578
})
25792579
ct.waitForWorkload()
25802580
},

‎pkg/cmd/roachtest/tests/latency_verifier.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ func (lv *latencyVerifier) noteHighwater(highwaterTime time.Time) {
116116
return
117117
}
118118

119-
if lv.targetSteadyLatency == 0 || latency < lv.targetSteadyLatency/2 {
119+
if lv.targetSteadyLatency == 0 || latency < lv.targetSteadyLatency {
120120
lv.latencyBecameSteady = true
121121
}
122122
if !lv.latencyBecameSteady {

‎pkg/roachprod/vm/ibm/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ go_library(
1818
"//pkg/roachprod/logger",
1919
"//pkg/roachprod/vm",
2020
"//pkg/roachprod/vm/flagstub",
21+
"//pkg/util/ctxgroup",
2122
"//pkg/util/randutil",
2223
"//pkg/util/retry",
2324
"//pkg/util/syncutil",

‎pkg/roachprod/vm/ibm/account_config.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ func (p *Provider) configureCloudAccountFull(cc *ibmCloudConfig) error {
121121
// Create a transit gateway for the account in the first region.
122122
tgID, err := p.getOrCreateTransitGateway(supportedRegions[0])
123123
if err != nil {
124-
return errors.Wrap(err, "failed to create transit gateway")
124+
return errors.Wrap(err, "failed to get or create transit gateway")
125125
}
126126

127127
for region := range p.regions {

0 commit comments

Comments
 (0)