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

roachtest: add a retry loop around network activity #31508

Merged
merged 1 commit into from
Oct 18, 2018

Conversation

BramGruneir
Copy link
Member

This is a first attempt to try to address #31127 and #31298 by putting a retry
loop around anything that does any downloading. We've seen a number of flakes
because of iffy networking and this should hopefully solve the problem.

If this doesn't fix the issues, the next step it to add some alternative
sources to sources.list.d.

Release note: None

@BramGruneir BramGruneir requested a review from tbg October 16, 2018 18:05
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@petermattis petermattis left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained


pkg/cmd/roachtest/hibernate.go, line 108 at r1 (raw file):

			// Build hibernate, this step involves some downloading, so it gets added
			// to the retry loop.

Starting the retry loop from scratch at this point seems unfortunate. The earlier parts seem reasonable to keep in the same loop, but I'm thinking this should be pulled out to a separate loop.

Copy link
Member Author

@BramGruneir BramGruneir left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained


pkg/cmd/roachtest/hibernate.go, line 108 at r1 (raw file):

Previously, petermattis (Peter Mattis) wrote…

Starting the retry loop from scratch at this point seems unfortunate. The earlier parts seem reasonable to keep in the same loop, but I'm thinking this should be pulled out to a separate loop.

Done.

Copy link
Collaborator

@petermattis petermattis left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained


pkg/cmd/roachtest/hibernate.go, line 111 at r2 (raw file):

		t.Status("building hibernate (without tests)")
		attempt = 0
		for r := retry.StartWithCtx(ctx, opts); r.Next(); {

I believe this can be for attempt, r := 0, retry.StartWithCtx(...); r.Next(); { ... }. Ditto for the loop above.

This is a first attempt to try to address cockroachdb#31127 and cockroachdb#31298 by putting a retry
loop around anything that does any downloading. We've seen a number of flakes
because of iffy networking and this should hopefully solve the problem.

If this doesn't fix the issues, the next step it to add some alternative
sources to sources.list.d.

Release note: None
@BramGruneir
Copy link
Member Author

bors r+

craig bot pushed a commit that referenced this pull request Oct 18, 2018
31508: roachtest: add a retry loop around network activity r=BramGruneir a=BramGruneir

This is a first attempt to try to address #31127 and #31298 by putting a retry
loop around anything that does any downloading. We've seen a number of flakes
because of iffy networking and this should hopefully solve the problem.

If this doesn't fix the issues, the next step it to add some alternative
sources to sources.list.d.

Release note: None

Co-authored-by: Bram Gruneir <bram@cockroachlabs.com>
Copy link
Member Author

@BramGruneir BramGruneir left a comment

Choose a reason for hiding this comment

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

TFTR!

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained


pkg/cmd/roachtest/hibernate.go, line 111 at r2 (raw file):

Previously, petermattis (Peter Mattis) wrote…

I believe this can be for attempt, r := 0, retry.StartWithCtx(...); r.Next(); { ... }. Ditto for the loop above.

Yep. Done.

@craig
Copy link
Contributor

craig bot commented Oct 18, 2018

Build succeeded

@craig craig bot merged commit a0115e6 into cockroachdb:master Oct 18, 2018
@BramGruneir BramGruneir deleted the hibernate-redux branch October 19, 2018 18:45
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