Skip to content

Conversation

amanpatel
Copy link
Contributor

Issue Description:

"Dumper" (pg_dump/ pg_restore) migrations can timeout on slow machines, or when dumps are fairly decent in size.

Proposed Solution

Remove the timeout for long lived processes such as pg_restore.

Default timeout is 60s, Symphony Process docs: https://symfony.com/doc/current/components/process.html#process-timeout

I didn't include any tests as it doesn't seem like this would need it. I encounter the 60second timeout on a GitLab instance that runs tests. During times when the shared server is being utilized a lot, the duration of the pg_restore process can last longer than 60s. I hope the team considers including this suggestion.

As shown in Symphony Process docs: https://symfony.com/doc/current/components/process.html#process-timeout

All Process spawned have a default timeout of 60 seconds. For a pg_dump script. 60 seconds might or might not be enough especially if this is run in a CI/CD envrionment where process contention (in shared or virtual hardware) is an issue.

Removing the timeout is great because running migrations could be seen as a long lived process by most users and developers.
@taylorotwell taylorotwell merged commit 2b95dd9 into laravel:8.x Oct 7, 2021
@GrahamCampbell GrahamCampbell changed the title Update SchemaState Process to remove timeout [8.x] Update SchemaState Process to remove timeout Oct 7, 2021
victorvilella pushed a commit to cdsistemas/framework that referenced this pull request Oct 12, 2021
As shown in Symphony Process docs: https://symfony.com/doc/current/components/process.html#process-timeout

All Process spawned have a default timeout of 60 seconds. For a pg_dump script. 60 seconds might or might not be enough especially if this is run in a CI/CD envrionment where process contention (in shared or virtual hardware) is an issue.

Removing the timeout is great because running migrations could be seen as a long lived process by most users and developers.
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