tracing: revert trace.mode default to legacy #59431
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
|
I looked at the profile for another tpccbench failure from last night that had similar symptoms: #59424 (comment). I don't have good intuition around whether |
irfansharif
approved these changes
Jan 26, 2021
|
bors r+ |
|
@irfansharif I'm in meetings for the rest of my work day, could you fix this up and get it in? |
|
bors r- |
|
Canceled. |
We are still seeing memory issues on tpccbench/nodes=6/cpu=16/multi-az which need to be investigated. Turn off background tracing while we do. Touches #58298. We're also reverting an earlier commit as part of this one (d252400). This revert is needed given we've not yet addressed an underlying bug (#59203). Release note: None
irfansharif
force-pushed
the
trace-mode-legacy
branch
from
26b9ade
to
7bd8a7c
Compare
|
bors r+ |
|
Build succeeded: |
This was referenced Feb 1, 2021
tbg
added a commit
to tbg/cockroach
that referenced
this pull request
Feb 8, 2021
We were previously propagating RPCs across boundaries only when they were verbose. We now propagate any span (except the noop span) regardless of current verbosity. This ensures that SQL (which always creates real, non-verbose spans by default) can trace the entirety of its operations. This enables the background collection of contention metadata in 21.1 and more generally, the idea of always-on tracing. Note that we had previously considered always-on tracing to mean avoiding the noop span altogether; due to memory retention issues[1][2] encountered we are taking a step back from that idea for the 21.1 release. [1]: cockroachdb#59370 [2]: cockroachdb#59431 Release note: None iasd Release note: None
tbg
added a commit
to tbg/cockroach
that referenced
this pull request
Feb 9, 2021
We were previously propagating RPCs across boundaries only when they were verbose. We now propagate any span (except the noop span) regardless of current verbosity. This ensures that SQL (which always creates real, non-verbose spans by default) can trace the entirety of its operations. This enables the background collection of contention metadata in 21.1 and more generally, the idea of always-on tracing. Note that we had previously considered always-on tracing to mean avoiding the noop span altogether; due to memory retention issues[1][2] encountered we are taking a step back from that idea for the 21.1 release. [1]: cockroachdb#59370 [2]: cockroachdb#59431 Release note: None
tbg
added a commit
to tbg/cockroach
that referenced
this pull request
Feb 15, 2021
We were previously propagating RPCs across boundaries only when they were verbose. We now propagate any span (except the noop span) regardless of current verbosity. This ensures that SQL (which always creates real, non-verbose spans by default) can trace the entirety of its operations. This enables the background collection of contention metadata in 21.1 and more generally, the idea of always-on tracing. Note that we had previously considered always-on tracing to mean avoiding the noop span altogether; due to memory retention issues[1][2] encountered we are taking a step back from that idea for the 21.1 release. [1]: cockroachdb#59370 [2]: cockroachdb#59431 Release note: None
tbg
added a commit
to tbg/cockroach
that referenced
this pull request
Feb 17, 2021
We were previously propagating RPCs across boundaries only when they were verbose. We now propagate any span (except the noop span) regardless of current verbosity. This ensures that SQL (which always creates real, non-verbose spans by default) can trace the entirety of its operations. This enables the background collection of contention metadata in 21.1 and more generally, the idea of always-on tracing. Note that we had previously considered always-on tracing to mean avoiding the noop span altogether; due to memory retention issues[1][2] encountered we are taking a step back from that idea for the 21.1 release. Somehow creating additional trace spans seems to have fouled up some tracing-based tests. I can't boil that ocean here, so I filed a follow-up issue: cockroachdb#60672 [1]: cockroachdb#59370 [2]: cockroachdb#59431 Release note: None
angelapwen
pushed a commit
to angelapwen/cockroach
that referenced
this pull request
Feb 17, 2021
We were previously propagating RPCs across boundaries only when they were verbose. We now propagate any span (except the noop span) regardless of current verbosity. This ensures that SQL (which always creates real, non-verbose spans by default) can trace the entirety of its operations. This enables the background collection of contention metadata in 21.1 and more generally, the idea of always-on tracing. Note that we had previously considered always-on tracing to mean avoiding the noop span altogether; due to memory retention issues[1][2] encountered we are taking a step back from that idea for the 21.1 release. [1]: cockroachdb#59370 [2]: cockroachdb#59431 Release note: None
tbg
added a commit
to tbg/cockroach
that referenced
this pull request
Feb 17, 2021
We were previously propagating RPCs across boundaries only when they were verbose. We now propagate any span (except the noop span) regardless of current verbosity. This ensures that SQL (which always creates real, non-verbose spans by default) can trace the entirety of its operations. This enables the background collection of contention metadata in 21.1 and more generally, the idea of always-on tracing. Note that we had previously considered always-on tracing to mean avoiding the noop span altogether; due to memory retention issues[1][2] encountered we are taking a step back from that idea for the 21.1 release. Somehow creating additional trace spans seems to have fouled up some tracing-based tests. I can't boil that ocean here, so I filed a follow-up issue: cockroachdb#60672 [1]: cockroachdb#59370 [2]: cockroachdb#59431 Release note: None
craig bot
pushed a commit
that referenced
this pull request
Feb 17, 2021
59992: tracing: propagate non-recording spans across rpc boundaries r=knz,irfansharif,raduberinde a=tbg We were previously propagating RPCs across boundaries only when they were verbose. We now propagate any span (except the noop span) regardless of current verbosity. This ensures that SQL (which always creates real, non-verbose spans by default) can trace the entirety of its operations. This enables the background collection of contention metadata in 21.1 and more generally, the idea of always-on tracing. Note that we had previously considered always-on tracing to mean avoiding the noop span altogether; due to memory retention issues[1][2] encountered we are taking a step back from that idea for the 21.1 release. [1]: #59370 [2]: #59431 Release note: None Co-authored-by: Tobias Grieger <tobias.b.grieger@gmail.com> Co-authored-by: Tobias Schottdorf <tobias.schottdorf@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We are still seeing memory issues on tpccbench/nodes=6/cpu=16/multi-az
which need to be investigated.
Turn off background tracing while we do.
Touches #58298.
Release note: None