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

Vendor libpq5.12.1 into Slugs - Heroku 18 only #936

Merged
merged 13 commits into from
Dec 11, 2019

Conversation

schneems
Copy link
Contributor

@schneems schneems commented Dec 9, 2019

Pick up from the work in #935 but removing Heroku-16 support until we can get that working in Docker locally

https://github.com/schneems/libpq_heroku_16_reproduction/tree/schneems/manually-download-install

(Note it's a branch)

Libpq is the client library used to communicate to postgresql. There was a bug in libpq 5.11 that essentially said if there is a problem with a configuration value, for example if you tried to set `pool` (which should be an integer) to a string, then the value would be silently ignored. In libpq 5.12 this behavior was "fixed" so that if you try to set a value to an incorrect type it will raise an error:

```
PG::UnableToSend: no connection to the server
```

Or 

```
CONNECTION_BAD
```

Here is the commit that changed the behavior postgres/postgres@e7a2217

The largest failure mode is when someone accidentally forgets to put a space after their ERB tag in `database.yml` but before their comment. For example:


```
connect_timeout: <%= ENV['DB_CONNECT_TIMEOUT'] || 5 %># seconds
```

Would be evaluated to a string of "5# seconds". This will work in libpq5.11 but fail in libpq5.12. This can be fixed by adding a space between `%>` and `# seconds`.

This change was rolled out on our stack image, but then rolled back to customers getting failures in the middle of the night when they had seemingly changed nothing. 

By vendoring this library in the slug, we have more control around when the update will happen. Rather than breaking changes when the stack image rolls out which might happen in the middle of the night for some developers, this change will only be applied as customers are deploying their apps. If they have a release phase then the error will likely be caught there and prevented from going into production.

This behavior introduces a helpful warning message along with URLs for getting more information around this issue.

We do not anticipate that a majority of applications will be affected by this change, however those that are affected experience large problems. The intent of rolling this change out in a buildpack is to minimize the exposure of apps that will trigger this failing condition.

Eventually libpq5.12.1 will be the default version on the stack. This change works to prepare developers for that change.
@schneems schneems force-pushed the schneems/libpq12-heroku-18-only branch 3 times, most recently from e2faeef to a8cb5cb Compare December 9, 2019 18:00
Adding a cache `fetch` method that will only trigger on the first time it is executed and sees that the key does not exist in the cache. Using this to only warn on the first deploy.
@schneems schneems merged commit cdae063 into master Dec 11, 2019
@schneems schneems deleted the schneems/libpq12-heroku-18-only branch December 11, 2019 19:24
@edmorley
Copy link
Member

Filed https://gus.my.salesforce.com/a07B00000079prxIAA for removing the vendoring now that the stack image has the latest libpq :-)

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