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

Upgrade PostgreSQL to v16 #3884

Merged
merged 6 commits into from
May 25, 2024
Merged

Conversation

enoch85
Copy link
Contributor

@enoch85 enoch85 commented May 6, 2024

Just tested, and seems to work!

Just tested, and seems to work!
Copy link

netlify bot commented May 6, 2024

Deploy Preview for teslamate ready!

Name Link
🔨 Latest commit 0dcabf6
🔍 Latest deploy log https://app.netlify.com/sites/teslamate/deploys/665106c13f789b00085ed6bb
😎 Deploy Preview https://deploy-preview-3884--teslamate.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Dulanic
Copy link
Collaborator

Dulanic commented May 6, 2024

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

@enoch85
Copy link
Contributor Author

enoch85 commented May 6, 2024

pg_uprade

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 pg_upgradenow just in case though, good advice! 👍

@enoch85
Copy link
Contributor Author

enoch85 commented May 6, 2024

OK, so it wasn't as easy as I thought. Will have to revert now, and do it properly with pg_dumpall instead.

Maybe the standard command recomended in documentation should be pg_dumpall instead of just pg_dump?

@enoch85
Copy link
Contributor Author

enoch85 commented May 6, 2024

I got this in the log now which I didn't before, don't know if it makes any sense? With pg_dumpI got no errors, but then maybe you don't need to recreate the tables either, since you're doing a dumpall? https://docs.teslamate.org/docs/maintenance/backup_restore#restore

2024-05-06 15:29:59.173 CEST [1] LOG:  starting PostgreSQL 16.2 (Debian 16.2-1.pgdg120+2) on x86_64-pc-linux-gnu, 
.......................
2024-05-06 22:38:33.499 CEST [86] ERROR:  role "teslamate_db_user" already exists
2024-05-06 22:38:33.499 CEST [86] STATEMENT:  CREATE ROLE teslamate_db_user;
2024-05-06 22:38:33.563 CEST [88] ERROR:  database "teslamate" already exists
2024-05-06 22:38:33.563 CEST [88] STATEMENT:  CREATE DATABASE teslamate WITH TEMPLATE = template0 ENCODING = 'UTF8' LOCALE = 'en_US.utf8';
``

@enoch85
Copy link
Contributor Author

enoch85 commented May 6, 2024

All good from me now anyway! 👍

@Dulanic
Copy link
Collaborator

Dulanic commented May 6, 2024

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.

@enoch85
Copy link
Contributor Author

enoch85 commented May 7, 2024

At least this PR could now be merged as it does what proposed - switch to PGSQL 16 and upgrade properly.

@parkr
Copy link

parkr commented May 7, 2024

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.

@enoch85
Copy link
Contributor Author

enoch85 commented May 7, 2024

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.

@enoch85
Copy link
Contributor Author

enoch85 commented May 7, 2024

@JakobLichterfeld What do you say, can I get your approval on this?

@JakobLichterfeld JakobLichterfeld added dependencies Pull requests that update a dependency file area:teslamate Related to TeslaMate core labels May 8, 2024
@JakobLichterfeld
Copy link
Collaborator

Thanks for your suggestion!
I knew, the time will come to update our db container. I'm with Dulanic, this need additional testing (our CI does not cover backup and restore for example), given the fact some users are still on very old Postgres versions (as we can see in the amount users who post their docker-compose.yml in issues).

@enoch85
Copy link
Contributor Author

enoch85 commented May 8, 2024

Thanks for your suggestion! I knew, the time will come to update our db container. I'm with Dulanic, this need additional testing (our CI does not cover backup and restore for example), given the fact some users are still on very old Postgres versions (as we can see in the amount users who post their docker-compose.yml in issues).

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. 🙂

@JakobLichterfeld
Copy link
Collaborator

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.

pg_dump and pg_dumpall are not the same. May you elaborate why changing makes sense? What are the consequences, what are the differences in our migration?

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.

@brianmay
Copy link
Collaborator

brianmay commented May 9, 2024

I am guessing that pg_dumpall will save some details (e.g. database user) that may not be recorded by pg_dump, but you probably do want to keep. Not tested though.

@enoch85
Copy link
Contributor Author

enoch85 commented May 9, 2024

From the docs (https://www.postgresql.org/docs/current/app-pgdump.html):

pg_dump only dumps a single database. To back up an entire cluster, or to back up global objects that are common to all databases in a cluster (such as roles and tablespaces), use pg_dumpall.

As I wrote above, I managed to upgrade to v16 with pg_dump but then @Dulanic made me worried I missed somthing by not running pg_dumpall - so I ended up with pg_dumpall even though both seemed to be working totally fine (I checked history from day one, everything was there afaics). My educated guess though is that pg_dump would be sufficient since you advice to create new tables and everything before importing. pg_dumpall will backup the whole PostgreSQL DB (all of them), including users and everything - and that's not needed since we are only looking for the data in one single database - teslamate. But, better safe than sorry, hence the commit.

@enoch85
Copy link
Contributor Author

enoch85 commented May 14, 2024

(Migrated) PostgreSQL 16 is still running fine. ;)

@swiffer
Copy link
Contributor

swiffer commented May 19, 2024

adrian did the same last year (bump to 15).

b79a454

i think it's fine to bump for new users.

@BowlesCR
Copy link

BowlesCR commented May 23, 2024

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 🤷

@parkr
Copy link

parkr commented May 23, 2024

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.

@enoch85
Copy link
Contributor Author

enoch85 commented May 23, 2024

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 dumpall really, but I got insecure as mentioned in previous posts.

@JakobLichterfeld
Copy link
Collaborator

In addition to my review findings: Please also adapt all other mentions in the docu.

@enoch85
Copy link
Contributor Author

enoch85 commented May 24, 2024

In addition to my review findings: Please also adapt all other mentions in the docu.

Done! 👍

@enoch85 enoch85 requested a review from JakobLichterfeld May 24, 2024 21:31
Copy link
Collaborator

@JakobLichterfeld JakobLichterfeld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ty!

@JakobLichterfeld JakobLichterfeld merged commit 95d899d into teslamate-org:master May 25, 2024
12 checks passed
@JakobLichterfeld
Copy link
Collaborator

CI is now updated as well, see #3916

@enoch85 enoch85 deleted the patch-1 branch May 25, 2024 07:30
@enoch85
Copy link
Contributor Author

enoch85 commented May 25, 2024

Great! 🎊

@cwanja
Copy link
Collaborator

cwanja commented Jun 19, 2024

Can we update the release notes to say something about "Documentation updates to support PostgreSQL 16". The release itself does not upgrade a users instance to PSQL 16. And I doubt most read the pull release. @JakobLichterfeld

@JakobLichterfeld
Copy link
Collaborator

Good point, will add it.

@greggitter
Copy link

Updated from version 12 to 16 today. For those using the teslamate manual install on debian (and maybe the same for Docker), this is the procedure I followed. I haven't seen any problems, if anyone knows why I shouldn't do it this way, please let me know. Thx.

sudo apt install postgresql-16 postgresql-client-16 -y
dpkg -l | grep postgresql (verify install)
sudo service postgresql stop
sudo pg_dropcluster 16 main --stop (drops the new empty 16 cluster)
sudo pg_upgradecluster 12 main
sudo service postgresql start
pg_lsclusters (verify 16 is active)
sudo pg_dropcluster 12 main (permanently deletes v12 cluster - VERIFY before doing this)
sudo apt remove postgresql-12 postgresql-client-12

Credit: https://www.youtube.com/watch?v=JvbbI_zxubQ

@JakobLichterfeld
Copy link
Collaborator

JakobLichterfeld commented Jun 19, 2024

if anyone knows why I shouldn't do it this way, please let me know.

First: manual is not the preferred setup. Additional No migration steps are done, which can lead to unexpected behavior. Therefore only follow TeslsaMate documentation

@greggitter
Copy link

OK, thanks...I actually looked and couldn't find the documentation for upgrading. All I found was this page https://docs.teslamate.org/docs/maintenance/upgrading_postgres but I'm not using docker.

@cwanja
Copy link
Collaborator

cwanja commented Jun 19, 2024

OK, thanks...I actually looked and couldn't find the documentation for upgrading. All I found was this page https://docs.teslamate.org/docs/maintenance/upgrading_postgres but I'm not using docker.

You won't find TeslaMate documentation on how to upgrade PostgreSQL manually because that is not our tool to own.

@greggitter
Copy link

Apologies, I guess that's not what I meant. I mean, I am using the MANUAL install of teslamate, so it obviously differs from the DOCKER install...yes? Maybe there are instructions for the "migration steps" somewhere but I didn't see them after searching for quite some time...including github issues.

@JakobLichterfeld
Copy link
Collaborator

JakobLichterfeld commented Jun 19, 2024

Maybe there are instructions for the "migration steps" somewhere but I didn't see them after searching for quite some time...including github issues.

See the restore step in above docu or direct link here:
https://docs.teslamate.org/docs/maintenance/backup_restore

@cwanja
Copy link
Collaborator

cwanja commented Jun 19, 2024

so it obviously differs from the DOCKER install...yes?

Yes

Maybe there are instructions for the "migration steps" somewhere but I didn't see them after searching for quite some time...including github issues.

Not sure what you mean by "migration steps"? And for what type of install? Docker is straight forward:

  1. Backup
  2. Shut down TeslaMate
  3. Remove PostgreSQL volume
  4. Update compose
  5. Restart and restore

@greggitter
Copy link

Thanks but I don't see how this applies since I have neither Docker nor Docker Compose installed. I have a native install in a proxmox lxc. There is no documentation for upgrading postgresql versions or elixir versions. Grafana has been updating itself via APT without issue. I've been running this for nearly a year. I appreciate the help.

@bmerciergallay
Copy link

bmerciergallay commented Jun 24, 2024

Is upgrading to pgsql 16 mandatory for 1.29.2 or not ? it's not as clear as it should

@enoch85
Copy link
Contributor Author

enoch85 commented Jun 24, 2024

Is upgrading to pgsql 16 mandatory for 1.29.2 or not ? it's not as clear as it should

It's not mandatory, but possible. IMHO you should upgrade.

@JakobLichterfeld
Copy link
Collaborator

Is upgrading to pgsql 16 mandatory for 1.29.2 or not ? it's not as clear as it should

If you do not upgrade and open an issue, first answer would be share your docker compose and then upgrade your postgres version. The only supported version is 16 at the time of writing. This means 12 is working fine, but nobody is testing it nor make assumptions about it.
TeslaMate is not in charge of dependencies, only testing with the versions stated. You can place any parallel here, starting from a car and an owner asking if it is allowed to use his tires without any profile or a cyclist asking if it allowed to run on a flat tire.

@bmerciergallay
Copy link

I strongly dispute the claim that having a postgresql > 12 and < 16 is like cycling with a flat tire ! 🤣
got it though...
I'll think about upgrading to 16 when the API landscape will be cleared and certain

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:teslamate Related to TeslaMate core dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.