Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cli: TestTenantZip flakes #87141

Closed
ajwerner opened this issue Aug 30, 2022 · 1 comment · Fixed by #98778
Closed

cli: TestTenantZip flakes #87141

ajwerner opened this issue Aug 30, 2022 · 1 comment · Fixed by #98778
Assignees
@aadityasondhi
Labels
A-observability-inf branch-release-23.1 Used to mark GA and release blockers, technical advisories, and bugs for 23.1 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. C-test-failure Broken test (automatically or manually discovered). GA-blocker

Comments

@ajwerner
Copy link
Contributor

ajwerner commented Aug 30, 2022 •
edited by cockroach-jira-scripts
Loading

Describe the problem

CI Run

Jira issue: CRDB-19174
@ajwerner ajwerner added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. C-test-failure Broken test (automatically or manually discovered). labels Aug 30, 2022
@ajwerner
Copy link
Contributor Author

Here's a fresh one: https://teamcity.cockroachdb.com/buildConfiguration/Cockroach_UnitTests_BazelUnitTests/6630099?showRootCauses=false&expandBuildChangesSection=true&expandBuildDeploymentsSection=true&expandBuildTestsSection=true

It happens because sometimes there are heap profiles or goroutine dumps. I think we need to disable that feature.

        -[node 1] 0 goroutine dumps found
        +[node 1] 2 goroutine dumps found
        +[node 1] writing dump... writing binary output: debug/nodes/1/goroutines/memprof.2022-09-26T16_17_33.984.70311048.pprof... done
        +[node 1] writing dump... writing binary output: debug/nodes/1/goroutines/memstats.2022-09-26T16_17_34.085.235642880.txt... done

@benbardin benbardin mentioned this issue Oct 3, 2022
Closed
@knz knz mentioned this issue Feb 5, 2023
Merged
craig bot pushed a commit that referenced this issue Feb 7, 2023
@pav-kv @knz
@nvanbenschoten @dt
Merge #96424 #96553 #96565 #96679
75eb88d 
96424: kvserver: factor out replica loading phase r=tbg a=pavelkalinnikov

This commit factors out Replica state loading and invariant checking so that it
is more consolidated rather than interleaved into Replica creation. Replica
creation path was loading this state, to post-factum assert that the in-memory
state matches the state in storage. Now this is a pre-requisite and input into
creating the Replica, and turning it from uninitialized to initialized state.

This change also eliminates the concept of "unloaded" Replica. Now the Replica
is created in a valid state (either initialized or uninitialized).

Touches #93898

96553: rpc,server: use node certs for shared-process tenant RPCs r=stevendanna a=knz

TLDR: This PR makes it possible to use regular node certs with shared-process multi-tenancy (i.e. skip creating client tenant certs). I think this will help `@aadityasondhi` 's work on `debug zip`.

Fixes #96215.
Epic: CRDB-23559

The PR also contains a few cleanup commits; it's a short change away from fixing  #87141 and #77173, which I intend to do in a separate PR.




96565: kv/logstore: avoid heap allocations around non-blocking sync waiter callback r=nvanbenschoten a=nvanbenschoten

This commit structures the non-blocking sync callback provided to the Raft log `SyncWaiterLoop` as a struct with a method that satisfies an interface (i.e. a functor) instead of an anonymous function. This provides more control over the memory layout of the callback and prevents individual fields from escaping to the heap. The change also provides the opportunity to pool the callback to avoid an additional heap allocation.

The change has the following effect on microbenchmarks:
```
name                                                 old time/op    new time/op    delta
ReplicaProposal/bytes=1.0_KiB,withFollower=false-10    40.1µs ± 3%    39.1µs ± 1%   -2.51%  (p=0.000 n=10+9)
ReplicaProposal/bytes=512_B,withFollower=false-10      38.2µs ± 2%    37.3µs ± 4%   -2.48%  (p=0.015 n=10+10)
ReplicaProposal/bytes=256_B,withFollower=false-10      37.1µs ± 1%    36.2µs ± 3%   -2.32%  (p=0.000 n=10+10)
ReplicaProposal/bytes=256_B,withFollower=true-10       52.2µs ± 1%    51.2µs ± 1%   -1.91%  (p=0.000 n=10+10)
ReplicaProposal/bytes=1.0_KiB,withFollower=true-10     58.5µs ± 2%    57.5µs ± 2%   -1.79%  (p=0.001 n=10+10)
ReplicaProposal/bytes=512_B,withFollower=true-10       53.8µs ± 2%    52.8µs ± 1%   -1.74%  (p=0.000 n=10+10)

name                                                 old speed      new speed      delta
ReplicaProposal/bytes=512_B,withFollower=false-10    13.4MB/s ± 2%  13.7MB/s ± 4%   +2.57%  (p=0.016 n=10+10)
ReplicaProposal/bytes=1.0_KiB,withFollower=false-10  25.5MB/s ± 2%  26.2MB/s ± 1%   +2.57%  (p=0.000 n=10+9)
ReplicaProposal/bytes=256_B,withFollower=false-10    6.91MB/s ± 1%  7.07MB/s ± 3%   +2.36%  (p=0.000 n=10+10)
ReplicaProposal/bytes=256_B,withFollower=true-10     4.90MB/s ± 1%  5.00MB/s ± 1%   +1.96%  (p=0.000 n=10+10)
ReplicaProposal/bytes=1.0_KiB,withFollower=true-10   17.5MB/s ± 2%  17.8MB/s ± 1%   +1.82%  (p=0.001 n=10+10)
ReplicaProposal/bytes=512_B,withFollower=true-10     9.52MB/s ± 2%  9.69MB/s ± 1%   +1.76%  (p=0.000 n=10+10)

name                                                 old alloc/op   new alloc/op   delta
ReplicaProposal/bytes=256_B,withFollower=false-10      14.6kB ± 0%    12.8kB ± 0%  -12.73%  (p=0.000 n=10+10)
ReplicaProposal/bytes=256_B,withFollower=true-10       35.0kB ± 1%    30.6kB ± 1%  -12.59%  (p=0.000 n=10+10)
ReplicaProposal/bytes=512_B,withFollower=true-10       42.9kB ± 0%    38.6kB ± 1%  -10.04%  (p=0.000 n=8+10)
ReplicaProposal/bytes=512_B,withFollower=false-10      18.5kB ± 2%    16.8kB ± 1%   -9.19%  (p=0.000 n=10+10)
ReplicaProposal/bytes=1.0_KiB,withFollower=true-10     60.6kB ± 1%    55.9kB ± 1%   -7.76%  (p=0.000 n=10+10)
ReplicaProposal/bytes=1.0_KiB,withFollower=false-10    27.5kB ± 2%    25.6kB ± 2%   -7.06%  (p=0.000 n=10+10)

name                                                 old allocs/op  new allocs/op  delta
ReplicaProposal/bytes=512_B,withFollower=false-10        70.0 ± 0%      61.6 ± 1%  -12.00%  (p=0.000 n=10+10)
ReplicaProposal/bytes=256_B,withFollower=false-10        69.0 ± 0%      61.0 ± 0%  -11.59%  (p=0.000 n=10+10)
ReplicaProposal/bytes=1.0_KiB,withFollower=false-10      73.0 ± 0%      65.0 ± 0%  -10.96%  (p=0.002 n=8+10)
ReplicaProposal/bytes=256_B,withFollower=true-10          179 ± 0%       161 ± 0%  -10.21%  (p=0.000 n=10+7)
ReplicaProposal/bytes=512_B,withFollower=true-10          181 ± 1%       162 ± 0%  -10.11%  (p=0.000 n=9+10)
ReplicaProposal/bytes=1.0_KiB,withFollower=true-10        186 ± 0%       168 ± 0%   -9.84%  (p=0.000 n=9+10)
```

Release note: None
Epic: None

96679: backupccl: only backup active tenants r=dt a=dt

Release note: none.
Epic: none.
Fixes: #89997.

Co-authored-by: Pavel Kalinnikov <[email protected]>
Co-authored-by: Raphael 'kena' Poss <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: David Taylor <[email protected]>
@dhartunian dhartunian added GA-blocker branch-release-23.1 Used to mark GA and release blockers, technical advisories, and bugs for 23.1 labels Mar 13, 2023
aadityasondhi added a commit to aadityasondhi/cockroach that referenced this issue Mar 16, 2023
@aadityasondhi
cli: unskip test tenant zip test
f6e62c9 
This patch unskips and re-records the datadriven `TestTenantZip` as it
was fixed in cockroachdb#96553, but was not unskipped or recorded. The test was run
locally using `--stress` and did not flake:
```
101 runs so far, 0 failures, over 5m0s
```

Fixes cockroachdb#87141

Release note: None
@aadityasondhi aadityasondhi mentioned this issue Mar 16, 2023
Merged
craig bot pushed a commit that referenced this issue Mar 21, 2023
@jordanlewis @aadityasondhi
@erikgrinaker @knz
Merge #97677 #98778 #98830 #99100 #99106
5f5cf26 
97677: tsearch: add stemming and stopword elimination for several languages r=jordanlewis a=jordanlewis

Updates: #41288
Epic: CRDB-22357
First commit is #92966.

    This commit adds stopword elimination for text search. The languages
    supported are the same ones that Postgres does. The stopword lists were
    copied from Postgres commit e757080e041214cf6983e3e77ef01e83f1371d72.

    Also, add snowball stemming provided by the blevesearch snowball
    stemming library.

    Release note (sql change): add stemming and stopword eliminating text
    search configurations for English, Danish, Dutch, Finnish, French,
    German, Hungarian, Italian, Norwegian, Portuguese, Russian, Spanish,
    Swedish, and Turkish.

98778: cli: unskip test tenant zip test r=dhartunian a=aadityasondhi

This patch unskips and re-records the datadriven `TestTenantZip` as it was fixed in #96553, but was not unskipped or recorded. The test was run locally using `--stress` and did not flake:
```
101 runs so far, 0 failures, over 5m0s
```

Fixes #87141

Release note: None

98830: sqlinstance: add `binary_version` column to instances table r=knz,JeffSwenson a=healthy-pod

This code change adds a `binary_version` column to the instances table.

This is done by adding the column to the bootstrap schema for system.sql_instances,
and piggy-backing on the existing code for the V23_1_SystemRbrReadNew migration
that overwrites the live schema for this table by the bootstrap copy.

This redefinition of the meaning of the V23_1_SystemRbrReadNew is backward-incompatible
and is only possible because this commit is merged before the v23.1 branch is cut.

Release note: None
Epic: [CRDB-20860](https://cockroachlabs.atlassian.net/browse/CRDB-20860)

99100: kvserver: skip `TestReplicateQueueExpirationLeasesOnly` under deadlock r=erikgrinaker a=erikgrinaker

I give up.

Resolves #99015.

Epic: none
Release note: None

99106: server: avoid some log spam r=erikgrinaker a=knz

This change removes the following log spam:
```
could not run claimed job 102: no resumer is available for AUTO CONFIG RUNNER
```

Epic: CRDB-23559
Release note: None

Co-authored-by: Jordan Lewis <[email protected]>
Co-authored-by: Aaditya Sondhi <[email protected]>
Co-authored-by: healthy-pod <[email protected]>
Co-authored-by: Erik Grinaker <[email protected]>
Co-authored-by: Raphael 'kena' Poss <[email protected]>
@craig craig bot closed this as completed in f9ebb04 Mar 21, 2023
abarganier pushed a commit to abarganier/cockroach that referenced this issue Sep 20, 2023
@aadityasondhi @abarganier
cli: unskip test tenant zip test
4132436 
This patch unskips and re-records the datadriven `TestTenantZip` as it
was fixed in cockroachdb#96553, but was not unskipped or recorded. The test was run
locally using `--stress` and did not flake:
```
101 runs so far, 0 failures, over 5m0s
```

Fixes cockroachdb#87141

Release note: None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aadityasondhi aadityasondhi

Labels
A-observability-inf branch-release-23.1 Used to mark GA and release blockers, technical advisories, and bugs for 23.1 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. C-test-failure Broken test (automatically or manually discovered). GA-blocker
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

cli: unskip test tenant zip test aadityasondhi/cockroach
3 participants
@dhartunian @ajwerner @aadityasondhi