-
Notifications
You must be signed in to change notification settings - Fork 672
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
Upgrade PostgreSQL to v16 #3884
Conversation
Just tested, and seems to work!
✅ Deploy Preview for teslamate ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
While I agree changing from v15 to v16 makes sense, please be aware this has breaking changes and simply upgrading in many cases will cause breaks and you will need to do a pg_dumpall or use pg_uprade so we need to be aware if a user just upgrades their container, it may cause issues. https://www.postgresql.org/docs/16/release-16.html#RELEASE-16-MIGRATION |
Yeah that could be added to the documented steps of upgrading PostgreSQL as well. Want me to fix? Not 100% sure about the command, but will check. JFTR, I checked all my statistics from 2022 --> now, and they seem to work, everything is there afaik. Will do a |
OK, so it wasn't as easy as I thought. Will have to revert now, and do it properly with Maybe the standard command recomended in documentation should be |
I got this in the log now which I didn't before, don't know if it makes any sense? With
|
All good from me now anyway! 👍 |
Now you understand the pain 🤣 I personally run a separate postgres server as I have many containers connect so I just transition when I feel like it. pg_upgrade doesn't work too well with container postgres. |
At least this PR could now be merged as it does what proposed - switch to PGSQL 16 and upgrade properly. |
What are the benefits of 16 over 15? As a user with several years of data, I'd be hesitant to upgrade unless there were benefits. |
Well, newer = better. :) I haven't gone into details, but updating is the best protection against old and unsecure code. Since it's "easy" to upgrade, why not? I also think that the default for new installs should be the latest version. |
@JakobLichterfeld What do you say, can I get your approval on this? |
Thanks for your suggestion! |
Hmm, I don't see the issue here actually. You set a tag in your docker-compose.yaml, and as long as that's set, nothing will change - the version will stay the same. If you on the other hand put a specific version (in this case 16) in the docker-compose.yaml file, it will install with 16, and stay as 16 until you change it. This PR merly suggests that all new users should get 16 directly, instead of 15 as it is today. It won't update current users. I run PostgreSQL with your Docker setup, I don't have any external DB, and I managed to upgrade according to your documented instructions. Personally, I went from 14 to 16 in one go. 🙂 |
Obviously, you get me wrong. Current state is: some users didn't upgrade Postgres every, they stick with old Postgres version. When they post their docker-compose, we often redirect them to our upgrade guide to be future-proof on that end, even if unrelated to the issue they report. If we change official installation instructions, we must ensure the manual upgrade process runs smooth, in-depended on the version they come from, this can be Postgres 11 for example.
I would love to upgrade to Postgres 16, but we need to ensure all is working as expected, as, as said above, our CI does not cover these cases. |
I am guessing that |
From the docs (https://www.postgresql.org/docs/current/app-pgdump.html):
As I wrote above, I managed to upgrade to v16 with |
(Migrated) PostgreSQL 16 is still running fine. ;) |
adrian did the same last year (bump to 15). i think it's fine to bump for new users. |
Fwiw, I've been running on Postgres 16-alpine since mid-February with no issues beyond the requisite backup/restore to migrate. At risk of starting a tangential discussion, debates to be had on using the alpine variant vs the default bookworm (actually bookworm-slim in the Dockerfile)... Without digging into the numbers the size reduction of alpine (only ~200MB) might be negated by no longer sharing the bookworm-slim base layer between the app and DB. That said, our Grafana image is based on alpine as well, so 🤷 |
Seems useful to have new users start on the latest version. I'd recommend splitting this PR into (1) the version change and (2) the backup/restore documentation change. FWIW I did the upgrade to 16 (from 13) following the existing instructions (no dumpall) and that worked well for me. |
Yeah, worked flawlessly for me as well, so I don't see any reason to use |
In addition to my review findings: Please also adapt all other mentions in the docu. |
Done! 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ty!
CI is now updated as well, see #3916 |
Great! 🎊 |
Just tested, and seems to work!