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

Add rack-timeout using default settings #1265

Merged
merged 2 commits into from
Aug 13, 2015

Conversation

camallen
Copy link
Contributor

Supersedes #1055

Adds react timeout with a default of 15s to process a request. Also bump gem minor updates lock mysql2 gem minor updates that won't build. Hopefully this will help with the stuck puma thread while we figure out a way to fix it.

    
Adds react timeout with a default of 15s to process a request.  Also bump gem minor updates lock mysql2 gem minor updates that won't build
updated schema_plus_pg_indexes gem breaks old migration
@edpaget
Copy link
Contributor

edpaget commented Aug 13, 2015

What kind of error does rack::timeout throw without any rescue_from? I think the error it raises will be uncaught leading to a 50x error type. If we make sure the error is 408, the frontend has code to catch and retry.

@camallen
Copy link
Contributor Author

500's by the looks, we can catch it and return a different error but need to take care that we don't get stuck there...https://github.com/heroku/rack-timeout#errors

@camallen
Copy link
Contributor Author

what does the front end do on 500's now?

@edpaget
Copy link
Contributor

edpaget commented Aug 13, 2015

Shows Error: 0. I don't think its necessarily reasonable to retry on 50x requests since they indicate an internal server error, whereas certain 40x errors are recoverable.

@edpaget
Copy link
Contributor

edpaget commented Aug 13, 2015

I had a local branch where I was working on rescuing and returning. I think I got hung up figuring out tests for it.

@edpaget
Copy link
Contributor

edpaget commented Aug 13, 2015

Nevermind looks like I don't have it anymore. I don't mind merging this in and figuring out the proper handling later.

@camallen
Copy link
Contributor Author

I was just going to say let's get this in and see what happens. I think these stuck threads that would raise the timeout error never return anyway. they get stuck looking for a FUTEX that is never set so the associated client request ends up timing out i assume.

@camallen
Copy link
Contributor Author

I see this as a stop gap, if it's causing too many problems we can rip it out and look at another solution.

edpaget added a commit that referenced this pull request Aug 13, 2015
Add rack-timeout using default settings
@edpaget edpaget merged commit f2357fc into zooniverse:master Aug 13, 2015
@camallen camallen deleted the rack-timeout branch August 17, 2015 15:56
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.

2 participants