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

Work around problems with ports in t/cycle #52

Closed
wants to merge 2 commits into from

Conversation

SpamapS
Copy link
Member

@SpamapS SpamapS commented Nov 30, 2016

The comments in this code suggest that a busy server can run into
problems and that's why the code retries up to 20 times. I've raised
that to 30 to deal with frequent failures in Travis CI.

We also change the Retry to info, instead of debug, as that is useful
information for a system administrator while the daemon starts up, and
should not be relegated to debug-only logs.

This closes #50

@SpamapS
Copy link
Member Author

SpamapS commented Nov 30, 2016

Hm, t/cycle failed again. I think we may need to add some debugging help to travis output.

Copy link
Collaborator

@p-alik p-alik left a comment

Choose a reason for hiding this comment

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

I run several tests on vagrant-ubuntu-trusty-64 and 16.04 and only once failed:

cycle.server_startup().server_startup(many) [ failed ]
FAIL t/cycle (exit status: 1)

@p-alik
Copy link
Collaborator

p-alik commented Nov 30, 2016

Retrying binding(Address already in use) messages appear only by testing with make test because on default make runs with -jN.
make -j1 test never produces the messages.
I'm afraid simultaneously running tests cause cycle test failure.

The comments in this code suggest that a busy server can run into
problems and that's why the code retries up to 20 times. I've raised
that to 50 to deal with frequent failures in Travis CI. Unfortunately
additions to this value seem to have diminishing returns. Presumably
there are things that slip into window between checking if the port is
available and binding gearmand to it that don't let go at all.

We also change the Retry to info, instead of debug, as that is useful
information for a system administrator while the daemon starts up, and
should not be relegated to debug-only logs.

This closes #50
Copy link
Collaborator

@p-alik p-alik left a comment

Choose a reason for hiding this comment

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

may I ask you about artifacts relation to the PL?

@SpamapS
Copy link
Member Author

SpamapS commented Dec 3, 2016

I'm going to rethink the approach. I think this patch does reduce the incidence of the problem, but the goal is to eliminate it and I think I have a scheme for doing that. The artifacts change is just a config change to get artifacts stored so that when these tests fail we can download them and attempt to debug.

@SpamapS SpamapS closed this Dec 3, 2016
@p-alik
Copy link
Collaborator

p-alik commented Dec 4, 2016

The artifacts change is just a config change to get artifacts stored so that when these tests fail we can download them and attempt to debug.

That sounds good.

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.

test t/cycle is racey and fails periodically
2 participants