Skip to content

tracing: revert trace.mode default to legacy#59431

Merged
craig[bot] merged 1 commit intomasterfrom
trace-mode-legacy
Jan 26, 2021
Merged

tracing: revert trace.mode default to legacy#59431
craig[bot] merged 1 commit intomasterfrom
trace-mode-legacy

Conversation

@tbg
Copy link
Copy Markdown
Member

@tbg tbg commented Jan 26, 2021

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

@tbg tbg requested review from irfansharif and nvb January 26, 2021 15:34
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@aayushshah15
Copy link
Copy Markdown
Contributor

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 startSpanGeneric is consuming the amount of memory we expect, so thought I'd flag it here.

@irfansharif
Copy link
Copy Markdown
Contributor

bors r+

@tbg
Copy link
Copy Markdown
Member Author

tbg commented Jan 26, 2021

@irfansharif I'm in meetings for the rest of my work day, could you fix this up and get it in?

@irfansharif
Copy link
Copy Markdown
Contributor

bors r-

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jan 26, 2021

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
@tbg tbg requested a review from a team as a code owner January 26, 2021 17:58
@irfansharif irfansharif removed the request for review from a team January 26, 2021 17:58
@irfansharif
Copy link
Copy Markdown
Contributor

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jan 26, 2021

Build succeeded:

@craig craig bot merged commit cc12bbc into master Jan 26, 2021
@irfansharif irfansharif deleted the trace-mode-legacy branch January 26, 2021 18:41
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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants