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

testcluster: always wait for full replication #9624

Closed
wants to merge 1 commit into from
Closed

testcluster: always wait for full replication #9624

wants to merge 1 commit into from

Conversation

tamird
Copy link
Contributor

@tamird tamird commented Sep 29, 2016

This makes it considerably less likely that database creation will cause
replication to stall in our benchmarks.

The underlying cause remains unknown.

Updates #8081.


This change is Reviewable

This makes it considerably less likely that database creation will cause
replication to stall in our benchmarks.

The underlying cause remains unknown.

Updates #8081.
@tamird
Copy link
Contributor Author

tamird commented Sep 29, 2016

Well, this appears to make matters worse (by surfacing more instances of this bug). cc @petermattis (TestLeaseInfoRequest seems to trigger the problem on this branch). I'd run it under stress.

@petermattis
Copy link
Collaborator

I'll take a look tomorrow as I have a school event to attend tonight.

@@ -177,6 +177,12 @@ func StartTestCluster(t testing.TB, nodes int, args base.TestClusterArgs) *TestC
tc.stopper.AddCloser(stop.CloserFn(tc.stopServers))

tc.waitForStores(t)

// TODO(tamird): consider removing this when #8081 is addressed.
if err := tc.WaitForFullReplication(); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will never complete if args.ReplicationMode == base.ReplicationManual, which it is in TestLeaseInfoRequest. Try:

    if args.ReplicationMode == base.ReplicationAuto {
        if err := tc.WaitForFullReplication(); err != nil {
            t.Fatal(err)
        }
    }

petermattis added a commit to petermattis/cockroach that referenced this pull request Oct 26, 2016
petermattis added a commit to petermattis/cockroach that referenced this pull request Oct 26, 2016
petermattis added a commit to petermattis/cockroach that referenced this pull request Oct 26, 2016
@tamird tamird closed this Oct 26, 2016
@tamird tamird deleted the fix-bench branch October 26, 2016 20:27
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.

5 participants