Skip to content

storage: fix stress flake#39838

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
ajwerner:ajwerner/fix-stress-flake-for-TestDefaultConnectionDisruptionDoesNotInterfereWithSystemTraffic
Sep 3, 2019
Merged

storage: fix stress flake#39838
craig[bot] merged 1 commit intocockroachdb:masterfrom
ajwerner:ajwerner/fix-stress-flake-for-TestDefaultConnectionDisruptionDoesNotInterfereWithSystemTraffic

Conversation

@ajwerner
Copy link
Copy Markdown
Contributor

While stressing storage for #39643 I encountered
TestDefaultConnectionDisruptionDoesNotInterfereWithSystemTraffic
failing under stress. It also complained about the parent testing.T
having been failed when then using a child so fixed that too though
in a perhaps messy way.

--- FAIL: TestDefaultConnectionDisruptionDoesNotInterfereWithSystemTraffic (2.29s)
    --- PASS: TestDefaultConnectionDisruptionDoesNotInterfereWithSystemTraffic/initial_run (0.14s)
    client_test.go:1280: [NotLeaseHolderError] r1: replica (n1,s1):1 not lease holder; current lease is repl=(n3,s3):3 seq=4 start=1566532897.322355813,1 exp=1566532898.378802707,0 pro=1566532897.478824123,0
    --- FAIL: TestDefaultConnectionDisruptionDoesNotInterfereWithSystemTraffic/after_restart (1.02s)
        testing.go:820: test executed panic(nil) or runtime.Goexit: subtest may have called FailNow on a parent test

Release note: None

@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@ajwerner ajwerner force-pushed the ajwerner/fix-stress-flake-for-TestDefaultConnectionDisruptionDoesNotInterfereWithSystemTraffic branch 3 times, most recently from aab9feb to dab92c6 Compare August 23, 2019 05:18
Copy link
Copy Markdown
Contributor

@nvb nvb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 2 of 2 files at r1.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @ajwerner)


pkg/storage/client_raft_test.go, line 4449 at r1 (raw file):

			}
		}
		// Use SucceedsSoon for deal rare stress cases where the transfer may fail.

s/deal //

While stressing storage for cockroachdb#39643 I encountered
TestDefaultConnectionDisruptionDoesNotInterfereWithSystemTraffic
failing under stress. It also complained about the parent testing.T
having been failed when then using a child so fixed that too though
in a perhaps messy way.

```
--- FAIL: TestDefaultConnectionDisruptionDoesNotInterfereWithSystemTraffic (2.29s)
    --- PASS: TestDefaultConnectionDisruptionDoesNotInterfereWithSystemTraffic/initial_run (0.14s)
    client_test.go:1280: [NotLeaseHolderError] r1: replica (n1,s1):1 not lease holder; current lease is repl=(n3,s3):3 seq=4 start=1566532897.322355813,1 exp=1566532898.378802707,0 pro=1566532897.478824123,0
    --- FAIL: TestDefaultConnectionDisruptionDoesNotInterfereWithSystemTraffic/after_restart (1.02s)
        testing.go:820: test executed panic(nil) or runtime.Goexit: subtest may have called FailNow on a parent test
```

Release note: None
@ajwerner ajwerner force-pushed the ajwerner/fix-stress-flake-for-TestDefaultConnectionDisruptionDoesNotInterfereWithSystemTraffic branch from dab92c6 to 1eaa87d Compare September 3, 2019 14:31
Copy link
Copy Markdown
Contributor Author

@ajwerner ajwerner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TFTR

bors r+

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @nvanbenschoten)


pkg/storage/client_raft_test.go, line 4449 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

s/deal //

Done.

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Sep 3, 2019

Build failed

@ajwerner
Copy link
Copy Markdown
Contributor Author

ajwerner commented Sep 3, 2019

Flaked on #40361

bors r+

craig bot pushed a commit that referenced this pull request Sep 3, 2019
39832: changefeedccl: allow base64-encoded client certificate r=rolandcrosby a=rolandcrosby

Adds `client_cert` and `client_key` options to the `kafka://`
changefeed URI scheme. Works like the existing `ca_cert` option:
the user base64-encodes the contents of a PEM certificate and
private key, and passes those base64 values as parameters in the
Kafka URI.

Fixes #39817.

Release note (enterprise change): Client certificates are now
supported for Kafka changefeed authentication.

39838: storage: fix stress flake r=ajwerner a=ajwerner

While stressing storage for #39643 I encountered
TestDefaultConnectionDisruptionDoesNotInterfereWithSystemTraffic
failing under stress. It also complained about the parent testing.T
having been failed when then using a child so fixed that too though
in a perhaps messy way.

```
--- FAIL: TestDefaultConnectionDisruptionDoesNotInterfereWithSystemTraffic (2.29s)
    --- PASS: TestDefaultConnectionDisruptionDoesNotInterfereWithSystemTraffic/initial_run (0.14s)
    client_test.go:1280: [NotLeaseHolderError] r1: replica (n1,s1):1 not lease holder; current lease is repl=(n3,s3):3 seq=4 start=1566532897.322355813,1 exp=1566532898.378802707,0 pro=1566532897.478824123,0
    --- FAIL: TestDefaultConnectionDisruptionDoesNotInterfereWithSystemTraffic/after_restart (1.02s)
        testing.go:820: test executed panic(nil) or runtime.Goexit: subtest may have called FailNow on a parent test
```

Release note: None

Co-authored-by: Roland Crosby <roland@cockroachlabs.com>
Co-authored-by: Andrew Werner <ajwerner@cockroachlabs.com>
@craig
Copy link
Copy Markdown
Contributor

craig bot commented Sep 3, 2019

Build succeeded

@craig craig bot merged commit 1eaa87d into cockroachdb:master Sep 3, 2019
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.

3 participants