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.

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

storage: fix stress flake #39838

Conversation

ajwerner
Copy link
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
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
Member

@nvanbenschoten nvanbenschoten 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
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
Contributor

craig bot commented Sep 3, 2019

Build failed

@ajwerner
Copy link
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
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