util/tracing: segfault in (*Span).Finish somewhere in sql/TestTrace
#57875
Labels
A-tracing
Relating to tracing in CockroachDB.
C-bug
Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
S-2-temp-unavailability
Temp crashes or other availability problems. Can be worked around or resolved by restarting.
Comments
|
Hi @knz, please add a C-ategory label to your issue. Check out the label system docs. While you're here, please consider adding an A- label to help keep our repository tidy. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
knz
added
A-tracing
knz
changed the title
(*trace).Finish() somewhere in sql/TestTrace(*Span).Finish somewhere in sql/TestTrace
|
I've also seen a couple of panics similar to it. |
|
Looking. |
craig bot
pushed a commit
that referenced
this issue
Dec 15, 2020
57879: sql: make writes to `system.eventlog` conditional r=tbg a=knz This patch is meant to help recovering partial availability in clusters where the `system.eventlog` table / range are unsavailable. Prior to this patch, when any SQL action was causing a notable event, that event would be written transactionally (in the same transaction) to the table `system.eventlog`. If that table happened to be unavailable, the action would not complete. This was true of even basic operations like changing a cluster setting, changing privileges on unrelated tables, etc. This patch changes that by introducing a new cluster setting `server.eventlog.enabled` to make these writes conditional. Release note (general change): The new cluster setting `server.eventlog.enabled` controls whether notable events are also written to the table `system.eventlog`. Its default value is `true`. Changing this cluster setting can help recovering partial cluster availability when the `system.eventlog` table becomes unavailable. Note that even when `false`, notable events are still propagated to the logging system, where they can be e.g. redirected to files. 57913: build: move `.pb.go` contents munging from `make` to `protoc-gen-gogoroach` r=rickystewart a=rickystewart This worked fine, but it complicated #56067 because [the standard Go protobuf Bazel support](https://github.com/bazelbuild/rules_go/blob/master/proto/core.rst#go_proto_library) doesn't easily allow us to just inject a random `sed` command into the middle of the (`.proto` -> `.pb.go` -> compiled Go library) pipeline. With this logic in the actual `protoc` plugin proper, we can then safely use Bazel's built-in stuff without a lot of monkey-patching or code duplication. Release note: None 57942: tracing: fix child span optimization r=knz a=tbg When given a context with a non-recording (but real) Span, we would return the incoming context. This would lead to an extra premature call to `Finish()` and subsequent use-after-Finish of the Span, which can blow up and/or hang, most of the time within net/trace code. Prior to this commit, the crash reproduced readily (within seconds) via ``` make stress PKG=./pkg/sql TESTS=TestTrace ``` and I had no issues for ~10 minutes with this commit. Fixes #57875. Release note: None Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net> Co-authored-by: Ricky Stewart <ricky@cockroachlabs.com> Co-authored-by: Tobias Grieger <tobias.b.grieger@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Found in CI in an unrelated PR: https://teamcity.cockroachdb.com/buildConfiguration/Cockroach_UnitTests_Test/2516403
It also repros readily with
make stress PKG=./pkg/sql TESTS=TestTracecc @tbg for triage.
The text was updated successfully, but these errors were encountered: