Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
95436: storage: rely on pebble for log message in case of write stall r=joshimhoff a=joshimhoff

Part of #67856. Haven't added additional context re: op type & size of write to `pebble.DiskSlowInfo` but will do that shortly. With this change, if we make changes to how much context is included in `pebble.DiskSlowInfo`, we will not need to make a CRDB change to have the context in the CRDB logs. Note also that I will adjust the pebble String method to be less accusatory to disks ("disk slow" -> "write slow").

See below for log message with this commit in.

```
$ ./dev test pkg/storage -v --filter='TestPebbleMetricEventListener' --show-logs
$ bazel test pkg/storage:all --test_env=GOTRACEBACK=all --test_filter=TestPebbleMetricEventListener --test_arg -test.v --test_arg -show-logs --test_sharding_strategy=disabled --test_output all
Starting local Bazel server and connecting to it...
INFO: Invocation ID: a4365f4b-9190-4ef3-8a54-de3b4c0c46b7
INFO: Analyzed 3 targets (626 packages loaded, 16556 targets configured).
INFO: Found 2 targets and 1 test target...
INFO: From Testing //pkg/storage:storage_test:
==================== Test output for //pkg/storage:storage_test:
initialized metamorphic constant "span-reuse-rate" with value 58
initialized metamorphic constant "mvcc-value-disable-simple-encoding" with value true
initialized metamorphic constant "mvcc-max-iters-before-seek" with value 2
initialized metamorphic constant "span-reuse-rate" with value 39
initialized metamorphic constant "mvcc-value-disable-simple-encoding" with value true
initialized metamorphic constant "mvcc-max-iters-before-seek" with value 0
=== RUN   TestPebbleMetricEventListener
    test_log_scope.go:76: test logs captured to: /private/var/tmp/_bazel_joshimhoff/69f147d3e5e8d9a9bc7bc343361a1a94/sandbox/darwin-sandbox/6/execroot/com_github_cockroachdb_cockroach/_tmp/88d640957a51906868fc787a6e8a38b4/logTestPebbleMetricEventListener440509452
E230118 21:01:15.941881 28 storage/pebble.go:1001  [-] 1  write stall detected: disk slowness detected: write to file  has been ongoing for 70.0s
    pebble_test.go:356: -- test log scope end --
--- PASS: TestPebbleMetricEventListener (0.05s)
PASS
================================================================================
INFO: Elapsed time: 29.194s, Critical Path: 8.66s
INFO: 7 processes: 1 internal, 6 darwin-sandbox.
INFO: Build completed successfully, 7 total actions
//pkg/storage:storage_test                                               PASSED in 0.6s

Executed 1 out of 1 test: 1 test passes.
INFO: Build completed successfully, 7 total actions
```

**storage: rely on pebble for log message in case of write stall**

In case of a write stall, CRDB logs out details re: the write stall. By
default, CRDB logs at fatal level & then crashes. Before this commit, tho the
pebble event had a String method, it was not used in the CRDB log message.
This means adding additional context to the pebble event will not add the
context to the CRDB logs, unless a CRDB change is made in addition to a pebble
change. With this commit, the log message calls the pebble event's String
method.

Release note (ops change): CRDB log message in case of write stall has been
adjusted slightly.

95697: cloud/kubernetes: update to v22.2.3 r=celiala a=cameronnunez

Part of https://cockroachlabs.atlassian.net/browse/REL-248

Release note: None

95698: roachtest: update version map for 22.2.3 r=rail,renatolabs a=cameronnunez

Part of: https://cockroachlabs.atlassian.net/browse/REL-248

Release note: None

95714: logictest: fix retry on query that returns a fleeting error r=cucaroach a=cucaroach

Fixes: #95664
Release note: None
Epic: CRDB-20535

95751: cdc: add transformations to create changefeed telemetry r=jayshrivastava a=jayshrivastava

Previously, there were no telemetry events emitted to track the usage of changefeed transformations. This change adds the `transformation` field to the `CREATE CHANGEFEED` telemetry log event to track that. Example:
```
{
  "Timestamp": 1674574083953989000, "EventType": "create_changefeed",
  "Description": "CREATE CHANGEFEED INTO 'gcpubsub://testfeed?region=testfeedRegion' AS SELECT b FROM foo",
  "SinkType": "gcpubsub", "NumTables": 1, "Resolved": "no", "Transformation": true
}
```

Epic: CRDB-17161
Closes: #95126

Release note: None

95784: dev: in `doctor`, include output of failed `git submodule` invocations r=healthy-pod a=rickystewart

Epic: none
Release note: None

Co-authored-by: Josh Imhoff <[email protected]>
Co-authored-by: Cameron Nunez <[email protected]>
Co-authored-by: Tommy Reilly <[email protected]>
Co-authored-by: Jayant Shrivastava <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
  • Loading branch information
6 people committed Jan 24, 2023
7 parents 27019cc + c48e3c7 + 0cf507f + 0038505 + 54e13d1 + 974177e + 7004ccd commit 64b182c
Show file tree
Hide file tree
Showing 43 changed files with 146 additions and 59 deletions.
2 changes: 1 addition & 1 deletion cloud/kubernetes/bring-your-own-certs/client.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ spec:
serviceAccountName: cockroachdb
containers:
- name: cockroachdb-client
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
# Keep a pod open indefinitely so kubectl exec can be used to get a shell to it
# and run cockroach client commands, such as cockroach sql, cockroach node status, etc.
command:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
imagePullPolicy: IfNotPresent
# TODO: Change these to appropriate values for the hardware that you're running. You can see
# the resources that can be allocated on each of your Kubernetes nodes by running:
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/client-secure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ spec:
mountPath: /cockroach-certs
containers:
- name: cockroachdb-client
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
imagePullPolicy: IfNotPresent
volumeMounts:
- name: client-certs
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/cluster-init-secure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ spec:
mountPath: /cockroach-certs
containers:
- name: cluster-init
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
imagePullPolicy: IfNotPresent
volumeMounts:
- name: client-certs
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/cluster-init.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ spec:
spec:
containers:
- name: cluster-init
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
imagePullPolicy: IfNotPresent
command:
- "/cockroach/cockroach"
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/cockroachdb-statefulset-secure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
imagePullPolicy: IfNotPresent
# TODO: Change these to appropriate values for the hardware that you're running. You can see
# the resources that can be allocated on each of your Kubernetes nodes by running:
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/cockroachdb-statefulset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
imagePullPolicy: IfNotPresent
# TODO: Change these to appropriate values for the hardware that you're running. You can see
# the resources that can be allocated on each of your Kubernetes nodes by running:
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/multiregion/client-secure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ spec:
serviceAccountName: cockroachdb
containers:
- name: cockroachdb-client
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
imagePullPolicy: IfNotPresent
volumeMounts:
- name: client-certs
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/multiregion/cluster-init-secure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ spec:
serviceAccountName: cockroachdb
containers:
- name: cluster-init
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
imagePullPolicy: IfNotPresent
volumeMounts:
- name: client-certs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
imagePullPolicy: IfNotPresent
ports:
- containerPort: 26257
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ spec:
name: cockroach-env
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
imagePullPolicy: IfNotPresent
# TODO: Change these to appropriate values for the hardware that you're running. You can see
# the resources that can be allocated on each of your Kubernetes nodes by running:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ spec:
hostNetwork: true
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
imagePullPolicy: IfNotPresent
# TODO: If you configured taints to give CockroachDB exclusive access to nodes, feel free
# to remove the requests and limits sections. If you didn't, you'll need to change these to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
imagePullPolicy: IfNotPresent
# TODO: If you configured taints to give CockroachDB exclusive access to nodes, feel free
# to remove the requests and limits sections. If you didn't, you'll need to change these to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ spec:
- name: cockroachdb
# NOTE: Always use the most recent version of CockroachDB for the best
# performance and reliability.
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
imagePullPolicy: IfNotPresent
# TODO: Change these to appropriate values for the hardware that you're running. You can see
# the resources that can be allocated on each of your Kubernetes nodes by running:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ spec:
- name: cockroachdb
# NOTE: Always use the most recent version of CockroachDB for the best
# performance and reliability.
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
imagePullPolicy: IfNotPresent
# TODO: Change these to appropriate values for the hardware that you're running. You can see
# the resources that can be allocated on each of your Kubernetes nodes by running:
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/v1.6/client-secure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ spec:
mountPath: /cockroach-certs
containers:
- name: cockroachdb-client
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
imagePullPolicy: IfNotPresent
volumeMounts:
- name: client-certs
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/v1.6/cluster-init-secure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ spec:
mountPath: /cockroach-certs
containers:
- name: cluster-init
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
imagePullPolicy: IfNotPresent
volumeMounts:
- name: client-certs
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/v1.6/cluster-init.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ spec:
spec:
containers:
- name: cluster-init
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
imagePullPolicy: IfNotPresent
command:
- "/cockroach/cockroach"
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/v1.6/cockroachdb-statefulset-secure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
imagePullPolicy: IfNotPresent
ports:
- containerPort: 26257
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/v1.6/cockroachdb-statefulset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
imagePullPolicy: IfNotPresent
ports:
- containerPort: 26257
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/v1.7/client-secure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ spec:
mountPath: /cockroach-certs
containers:
- name: cockroachdb-client
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
imagePullPolicy: IfNotPresent
volumeMounts:
- name: client-certs
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/v1.7/cluster-init-secure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ spec:
mountPath: /cockroach-certs
containers:
- name: cluster-init
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
imagePullPolicy: IfNotPresent
volumeMounts:
- name: client-certs
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/v1.7/cluster-init.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ spec:
spec:
containers:
- name: cluster-init
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
imagePullPolicy: IfNotPresent
command:
- "/cockroach/cockroach"
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/v1.7/cockroachdb-statefulset-secure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
imagePullPolicy: IfNotPresent
ports:
- containerPort: 26257
Expand Down
2 changes: 1 addition & 1 deletion cloud/kubernetes/v1.7/cockroachdb-statefulset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ spec:
topologyKey: kubernetes.io/hostname
containers:
- name: cockroachdb
image: cockroachdb/cockroach:v22.2.2
image: cockroachdb/cockroach:v22.2.3
imagePullPolicy: IfNotPresent
ports:
- containerPort: 26257
Expand Down
3 changes: 3 additions & 0 deletions docs/generated/eventlog.md
Original file line number Diff line number Diff line change
Expand Up @@ -2711,6 +2711,9 @@ successfully starts running. Failed CREATE statements will show up as
ChangefeedFailed events.


| Field | Description | Sensitive |
|--|--|--|
| `Transformation` | | no |


#### Common fields
Expand Down
7 changes: 4 additions & 3 deletions pkg/ccl/changefeedccl/changefeed_stmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ func changefeedPlanHook(
p.ExtendedEvalContext().Descs.ReleaseAll(ctx)

telemetry.Count(`changefeed.create.core`)
logChangefeedCreateTelemetry(ctx, jr)
logChangefeedCreateTelemetry(ctx, jr, changefeedStmt.Select != nil)

var err error
for r := getRetry(ctx); r.Next(); {
Expand Down Expand Up @@ -325,7 +325,7 @@ func changefeedPlanHook(
return err
}

logChangefeedCreateTelemetry(ctx, jr)
logChangefeedCreateTelemetry(ctx, jr, changefeedStmt.Select != nil)

select {
case <-ctx.Done():
Expand Down Expand Up @@ -1261,7 +1261,7 @@ func getChangefeedTargetName(
return desc.GetName(), nil
}

func logChangefeedCreateTelemetry(ctx context.Context, jr *jobs.Record) {
func logChangefeedCreateTelemetry(ctx context.Context, jr *jobs.Record, isTransformation bool) {
var changefeedEventDetails eventpb.CommonChangefeedEventDetails
if jr != nil {
changefeedDetails := jr.Details.(jobspb.ChangefeedDetails)
Expand All @@ -1270,6 +1270,7 @@ func logChangefeedCreateTelemetry(ctx context.Context, jr *jobs.Record) {

createChangefeedEvent := &eventpb.CreateChangefeed{
CommonChangefeedEventDetails: changefeedEventDetails,
Transformation: isTransformation,
}

log.StructuredEvent(ctx, createChangefeedEvent)
Expand Down
24 changes: 18 additions & 6 deletions pkg/ccl/changefeedccl/changefeed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7484,21 +7484,33 @@ func TestChangefeedCreateTelemetryLogs(t *testing.T) {

createLogs := checkCreateChangefeedLogs(t, beforeCreateSinkless.UnixNano())
require.Equal(t, 1, len(createLogs))
require.Equal(t, createLogs[0].SinkType, "core")
require.Equal(t, "core", createLogs[0].SinkType)
})

t.Run(`gcpubsub_sink_type with options`, func(t *testing.T) {
t.Run(`gcpubsub_sink_type_with_options`, func(t *testing.T) {
pubsubFeedFactory := makePubsubFeedFactory(s.Server, s.DB)
beforeCreatePubsub := timeutil.Now()
pubsubFeed := feed(t, pubsubFeedFactory, `CREATE CHANGEFEED FOR foo, bar WITH resolved="10s", no_initial_scan`)
defer closeFeed(t, pubsubFeed)

createLogs := checkCreateChangefeedLogs(t, beforeCreatePubsub.UnixNano())
require.Equal(t, 1, len(createLogs))
require.Equal(t, createLogs[0].SinkType, `gcpubsub`)
require.Equal(t, createLogs[0].NumTables, int32(2))
require.Equal(t, createLogs[0].Resolved, `10s`)
require.Equal(t, createLogs[0].InitialScan, `no`)
require.Equal(t, `gcpubsub`, createLogs[0].SinkType)
require.Equal(t, int32(2), createLogs[0].NumTables)
require.Equal(t, `10s`, createLogs[0].Resolved)
require.Equal(t, `no`, createLogs[0].InitialScan)
require.Equal(t, false, createLogs[0].Transformation)
})

t.Run(`with_transformation`, func(t *testing.T) {
pubsubFeedFactory := makePubsubFeedFactory(s.Server, s.DB)
beforeCreateWithTransformation := timeutil.Now()
pubsubFeed := feed(t, pubsubFeedFactory, `CREATE CHANGEFEED AS SELECT b FROM foo`)
defer closeFeed(t, pubsubFeed)

createLogs := checkCreateChangefeedLogs(t, beforeCreateWithTransformation.UnixNano())
require.Equal(t, 1, len(createLogs))
require.Equal(t, true, createLogs[0].Transformation)
})
}

Expand Down
7 changes: 7 additions & 0 deletions pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions pkg/cmd/dev/doctor.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,8 @@ You can install node with: ` + "`pkg install node`"
if _, err := os.Stat("bin/.submodules-initialized"); err == nil {
return ""
}
if _, err := d.exec.CommandContextSilent(ctx, "git", "submodule", "update", "--init", "--recursive"); err != nil {
return err.Error()
if output, err := d.exec.CommandContextSilent(ctx, "git", "submodule", "update", "--init", "--recursive"); err != nil {
return fmt.Sprintf("failed to run `git submodule update --init --recursive`: %+v: got output %s", err, string(output))
}
if err := d.os.MkdirAll("bin"); err != nil {
return err.Error()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@
"21.2": "21.1.12",
"22.1": "21.2.7",
"22.2": "22.1.6",
"23.1": "22.2.2"
"23.1": "22.2.3"
}
18 changes: 8 additions & 10 deletions pkg/sql/logictest/logic.go
Original file line number Diff line number Diff line change
Expand Up @@ -3123,12 +3123,7 @@ func (t *logicTest) verifyError(
) (bool, error) {
if expectErr == "" && expectErrCode == "" && err != nil {
t.maybeSkipOnRetry(err)
cont := t.unexpectedError(sql, pos, err)
if cont {
// unexpectedError() already reported via t.Errorf. no need for more.
err = nil
}
return cont, err
return t.unexpectedError(sql, pos, err)
}
if expectNotice != "" {
foundNotice := strings.Join(t.noticeBuffer, "\n")
Expand Down Expand Up @@ -3215,7 +3210,7 @@ func formatErr(err error) string {
// when -allow-prepare-fail is specified. The argument "sql" is "" to indicate the
// work is done on behalf of a statement, which always fail upon an
// unexpected error.
func (t *logicTest) unexpectedError(sql string, pos string, err error) bool {
func (t *logicTest) unexpectedError(sql string, pos string, err error) (bool, error) {
if *allowPrepareFail && sql != "" {
// This is a query and -allow-prepare-fail is set. Try to prepare
// the query. If prepare fails, this means we (probably) do not
Expand All @@ -3227,14 +3222,17 @@ func (t *logicTest) unexpectedError(sql string, pos string, err error) bool {
t.outf("\t-- fails prepare: %s", formatErr(err))
}
t.signalIgnoredError(err, pos, sql)
return true
return true, nil
}
if err := stmt.Close(); err != nil {
t.Errorf("%s: %s\nerror when closing prepared statement: %s", sql, pos, formatErr(err))
}
}
t.Errorf("%s: %s\nexpected success, but found\n%s", pos, sql, formatErr(err))
return false
// N.B. We return an error instead of calling t.Errorf because this query
// could be asking for a retry. We still use t.Errorf above because
// stmt.Close error is probably a sign of bigger issues and not
// something retryable.
return false, fmt.Errorf("%s: %s\nexpected success, but found\n%s", pos, sql, formatErr(err))
}

func (t *logicTest) execStatement(stmt logicStatement) (bool, error) {
Expand Down
Loading

0 comments on commit 64b182c

Please sign in to comment.