util: log metamorphic constants only with --show-logs#75065
Closed
irfansharif wants to merge 1 commit intocockroachdb:masterfrom
Closed
util: log metamorphic constants only with --show-logs#75065irfansharif wants to merge 1 commit intocockroachdb:masterfrom
irfansharif wants to merge 1 commit intocockroachdb:masterfrom
Conversation
Makes some test invocations less verbose. Release note: None
Member
Contributor
Author
|
(Bump.) |
stevendanna
reviewed
Jan 26, 2022
Collaborator
stevendanna
left a comment
There was a problem hiding this comment.
I'm not necessarily opposed to this, but is there a particular place that this came up as a problem? I worry that this would make it harder to do post-hoc debugging of CI failures caused by metamorphic constants.
stevendanna
added a commit
to stevendanna/cockroach
that referenced
this pull request
Feb 9, 2022
Commands such as `./dev test --verbose` might previously would produce
two log lines everytime it happens to have to call the genrule for
execgen targets:
```
INFO: From Executing genrule //pkg/sql/colexec/colexecwindow:gen-window-framer:
initialized metamorphic constant "span-reuse-rate" with value 10
```
This is because execgen has the tracing package in its dependency tree
```
> go list -f '{{ join .Deps "\n" }}' ./pkg/sql/colexec/execgen/ | grep tracing | head -1
github.com/cockroachdb/cockroach/pkg/util/tracing
```
and that package instantiates this metamorphic constant.
This PR sets COCKROACH_INTERNAL_DISABLE_METAMORPHIC_TESTING=true when
calling the generator so we don't get metamorphic constants enabled.
Something like cockroachdb#75065 would give us more complete coverage of commands
that might be generating noise.
Release note: None
Collaborator
|
Now that I'm using |
Contributor
Author
|
Steven did it better in #76309. |
craig bot
pushed a commit
that referenced
this pull request
Feb 10, 2022
76210: util/tracing: reduce allocations in EnsureForkSpan r=andreimatei a=andreimatei This patch pre-allocates capacity for the options slice in EnsureForkSpan. Before, we were reallocating the slice storage repeatedly, as it was growing from 0 to 1 and from 1 to 2 elements. This patch also simplifies, and slighly speeds up, EnsureChildSpan. This guy was using a sync.Pool for its options slice. That is not necessary, since the slice storage does not escape. In the benchmarks below, detached-child=false is EnsureChildSpan and detached-child=true is EnsureForkSpan. > make bench PKG=./pkg/util/tracing BENCHES=BenchmarkSpanCreation TESTFLAGS="-count=5 -cpu=1,2,4,8,16" name old time/op new time/op delta SpanCreation/detached-child=false 1.70µs ± 1% 1.61µs ± 1% -5.06% (p=0.008 n=5+5) SpanCreation/detached-child=false-2 1.17µs ± 7% 1.04µs ± 3% -10.70% (p=0.008 n=5+5) SpanCreation/detached-child=false-4 958ns ± 2% 893ns ±10% -6.76% (p=0.008 n=5+5) SpanCreation/detached-child=false-8 1.11µs ±38% 0.95µs ± 1% ~ (p=0.310 n=5+5) SpanCreation/detached-child=false-16 925ns ± 1% 919ns ± 1% ~ (p=0.286 n=4+5) SpanCreation/detached-child=true 2.27µs ± 2% 1.88µs ± 0% -17.27% (p=0.008 n=5+5) SpanCreation/detached-child=true-2 1.82µs ±10% 1.33µs ±22% -26.81% (p=0.008 n=5+5) SpanCreation/detached-child=true-4 1.29µs ± 2% 1.02µs ± 1% -21.35% (p=0.008 n=5+5) SpanCreation/detached-child=true-8 1.16µs ± 1% 0.91µs ± 5% -21.56% (p=0.008 n=5+5) SpanCreation/detached-child=true-16 1.14µs ± 2% 0.97µs ± 1% -15.08% (p=0.008 n=5+5) name old alloc/op new alloc/op delta SpanCreation/detached-child=false 288B ± 0% 288B ± 0% ~ (all equal) SpanCreation/detached-child=false-2 288B ± 0% 288B ± 0% ~ (all equal) SpanCreation/detached-child=false-4 288B ± 0% 288B ± 0% ~ (all equal) SpanCreation/detached-child=false-8 288B ± 0% 288B ± 0% ~ (all equal) SpanCreation/detached-child=false-16 288B ± 0% 288B ± 0% ~ (all equal) SpanCreation/detached-child=true 768B ± 0% 288B ± 0% -62.50% (p=0.008 n=5+5) SpanCreation/detached-child=true-2 768B ± 0% 288B ± 0% -62.50% (p=0.008 n=5+5) SpanCreation/detached-child=true-4 768B ± 0% 288B ± 0% -62.50% (p=0.008 n=5+5) SpanCreation/detached-child=true-8 768B ± 0% 288B ± 0% -62.50% (p=0.008 n=5+5) SpanCreation/detached-child=true-16 769B ± 0% 288B ± 0% -62.55% (p=0.008 n=5+5) name old allocs/op new allocs/op delta SpanCreation/detached-child=false 6.00 ± 0% 6.00 ± 0% ~ (all equal) SpanCreation/detached-child=false-2 6.00 ± 0% 6.00 ± 0% ~ (all equal) SpanCreation/detached-child=false-4 6.00 ± 0% 6.00 ± 0% ~ (all equal) SpanCreation/detached-child=false-8 6.00 ± 0% 6.00 ± 0% ~ (all equal) SpanCreation/detached-child=false-16 6.00 ± 0% 6.00 ± 0% ~ (all equal) SpanCreation/detached-child=true 16.0 ± 0% 6.0 ± 0% -62.50% (p=0.008 n=5+5) SpanCreation/detached-child=true-2 16.0 ± 0% 6.0 ± 0% -62.50% (p=0.008 n=5+5) SpanCreation/detached-child=true-4 16.0 ± 0% 6.0 ± 0% -62.50% (p=0.008 n=5+5) SpanCreation/detached-child=true-8 16.0 ± 0% 6.0 ± 0% -62.50% (p=0.008 n=5+5) SpanCreation/detached-child=true-16 16.0 ± 0% 6.0 ± 0% -62.50% (p=0.008 n=5+5) Release note: None 76308: changefeedccl: remove methods from testServerShim r=miretskiy a=stevendanna In #75852 these methods have recently been added to the underlying TestTenantInterface, so we no longer need to stub them out in this shim. Release note: None 76309: build: disable metamorphic constants when calling execgen r=irfansharif a=stevendanna Commands such as `./dev test --verbose` might previously produce two log lines everytime it happens to have to call the genrule for execgen targets: ``` INFO: From Executing genrule //pkg/sql/colexec/colexecwindow:gen-window-framer: initialized metamorphic constant "span-reuse-rate" with value 10 ``` This is because execgen has the tracing package in its dependency tree ``` > go list -f '{{ join .Deps "\n" }}' ./pkg/sql/colexec/execgen/ | grep tracing | head -1 github.com/cockroachdb/cockroach/pkg/util/tracing ``` and that package instantiates this metamorphic constant. This PR sets COCKROACH_INTERNAL_DISABLE_METAMORPHIC_TESTING=true when calling the generator so we don't get metamorphic constants enabled. Something like #75065 would give us more complete coverage of commands that might be generating noise. Release note: None 76338: jobs: De-flake TestTransientTxnErrors test. r=miretskiy a=miretskiy Fix rare, but silly flake in TestTransientTxnErrors where the `executeSchedules` go routine could run before we were ready. Fixes #65045 Release Notes: None Co-authored-by: Andrei Matei <andrei@cockroachlabs.com> Co-authored-by: Steven Danna <danna@cockroachlabs.com> Co-authored-by: Yevgeniy Miretskiy <yevgeniy@cockroachlabs.com>
RajivTS
pushed a commit
to RajivTS/cockroach
that referenced
this pull request
Mar 6, 2022
Commands such as `./dev test --verbose` might previously would produce
two log lines everytime it happens to have to call the genrule for
execgen targets:
```
INFO: From Executing genrule //pkg/sql/colexec/colexecwindow:gen-window-framer:
initialized metamorphic constant "span-reuse-rate" with value 10
```
This is because execgen has the tracing package in its dependency tree
```
> go list -f '{{ join .Deps "\n" }}' ./pkg/sql/colexec/execgen/ | grep tracing | head -1
github.com/cockroachdb/cockroach/pkg/util/tracing
```
and that package instantiates this metamorphic constant.
This PR sets COCKROACH_INTERNAL_DISABLE_METAMORPHIC_TESTING=true when
calling the generator so we don't get metamorphic constants enabled.
Something like cockroachdb#75065 would give us more complete coverage of commands
that might be generating noise.
Release note: None
This file contains hidden or 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
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.
Makes some test invocations less verbose.
Release note: None