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

Increase db timeout to 90s #2276

Merged
merged 1 commit into from
Jan 2, 2025
Merged

Conversation

Jongmassey
Copy link
Contributor

closes #2252

@Jongmassey Jongmassey marked this pull request as ready for review December 31, 2024 15:08
@bloodearnest
Copy link
Member

Do we know what the current HTTP request timeouts are?

The defaults for nginx and gunicorn are 60s and 30s respectively.

Naively, I would expect increasing the sqlite timeout to more that that to not solve the problem for the user.

Additionally, i suspect the lack of IMMEDIATE_MODE, which means many queries will not wait for the timeout anyway, means this won't have the desired effect?

@mikerkelly
Copy link
Contributor

mikerkelly commented Jan 2, 2025

Do we know what the current HTTP request timeouts are?

Good point. I think we're okay as they're higher than the 90s used in this PR.

nginx:

dokku3:~$ dokku nginx:show-config opencodelists | grep timeout
    keepalive_timeout   70;
    proxy_read_timeout 600;

I think proxy_read_timeout is the relevant one for the slow requests we're concerned about as they are reverse-proxied on to gunicorn. Static files use the default of 60s. I don't think the keep alive timeout is relevant here.

gunicorn:

timeout = 120

Additionally, i suspect the lack of IMMEDIATE_MODE, which means many queries will not wait for the timeout anyway, means this won't have the desired effect?

Quite possibly. As you brought up in #2251 changing SQLite configuration is likely a better fix but I don't think we know that for sure yet. Maybe there are multiple issues at play. That's #2268 but it hasn't been triaged yet as it was raised just before Christmas.

This PR is very cheap and may help, in some cases, depending what the specific issue actually is.

If the general issue gets fully resolved we might consider reducing this timeout again.

@mikerkelly
Copy link
Contributor

As a point of comparison for later monitoring, there were about 175 errors regular Sentry alerts for several Issues where requests fail with OperationalError: database is locked errors in October-December 2024.

@Jongmassey Jongmassey force-pushed the Jongmassey/increase-db-timeout-90s branch from b21303b to 3b69eb2 Compare January 2, 2025 10:28
@Jongmassey Jongmassey enabled auto-merge January 2, 2025 10:28
@Jongmassey Jongmassey merged commit a5de20d into main Jan 2, 2025
6 checks passed
@Jongmassey Jongmassey deleted the Jongmassey/increase-db-timeout-90s branch January 2, 2025 10:30
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.

Increase database timeout
3 participants