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

infra/specify postgres version to match when dumping and restoring #913

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

nhoening
Copy link
Contributor

Description

When dumping or restoring a database, FlexMeasures devs and hosters would have a problem, when the target database has a different version than the default local database. One needs to install the version of the target database locally, but pg_dump and pg_restore might still choose the default local version by default and then report a version mismatch as error.

Look & Feel

This PR adds a new CLI option --pg-version to the two commands flexmeasures db-ops dump and flexmeasures db-ops restore.

How to test

Dump the simulation database to file (it has PG 15, you might have PG 14 locally as default - which you can test with pg_dump --version.

Restore that dump to your local test database (set the db connection string in your ~/.flexmeasures.cfg first). I'm not sure this would run into the version mismatch, but you can see if restoring still works.

Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
@nhoening nhoening added this to the 0.18.0 milestone Dec 18, 2023
Signed-off-by: F.N. Claessen <felix@seita.nl>
Copy link
Contributor

@Flix6x Flix6x left a comment

Choose a reason for hiding this comment

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

Looks like the cluster argument isn't working nicely with my PostgreSQL version (13). I've suggested a fix. I also implemented a function that attempts to fetch the relevant PostgreSQL version by itself, and improved the error logging.

More info (e.g. on the format) at https://manpages.ubuntu.com/manpages/trusty/man1/pg_wrapper.1.html
"""
if pg_version:
return f"--cluster {pg_version}/main"
Copy link
Contributor

Choose a reason for hiding this comment

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

This didn't work for me. I got an Error: Invalid version 15 specified with --cluster. However, changing this line to return f"--version {pg_version}" made the dump work. I'm running PostgreSQL 13.5. I'm guessing you are running version 14, so perhaps you can test whether my remedy would suit you, too?

Copy link
Contributor

Choose a reason for hiding this comment

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

I just implemented this change in my last commit for your testing convenience. If it works for you, too, please consider refactoring (e.g. renaming) pg_cluster_arg_from_version.

Copy link
Contributor Author

@nhoening nhoening left a comment

Choose a reason for hiding this comment

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

I don't think one of your improvements does what you think it does.

More info (e.g. on the format) at https://manpages.ubuntu.com/manpages/trusty/man1/pg_wrapper.1.html
"""
if pg_version:
return f"--version {pg_version}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Flix6x Both pg_dump --help and pg_restore --help agree that --version leads to its very own outcome:

  -V, --version                output version information, then exit

And indeed, when I test flexmeasures db-ops dump --pg-version 15, the command says it created a dump file, but it actually did not. (as it exited, without an error, so our code assumed it did what we want).

You said "the cluster argument isn't working nicely with my PostgreSQL version (13)", but without further explanation. Right now, I don't know what that means. It worked with my Postgres version 14.

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, I failed to properly validate my solution. The error I get with return f"--cluster {pg_version}/main" is:

Exception happened during dump: Error: Invalid version 15.5 specified with --cluster

I'll revert my last commit. Then, we can try to work the issue I encounter with v13 (any suggestions?), or we could document that it only works for v14 or higher.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, according to the PostgreSQL docs (v13):

Because pg_dump is used to transfer data to newer versions of PostgreSQL, the output of pg_dump can be expected to load into PostgreSQL server versions newer than pg_dump's version. pg_dump can also dump from PostgreSQL servers older than its own version. (Currently, servers back to version 8.0 are supported.) However, pg_dump cannot dump from PostgreSQL servers newer than its own major version; it will refuse to even try, rather than risk making an invalid dump. Also, it is not guaranteed that pg_dump's output can be loaded into a server of an older major version — not even if the dump was taken from a server of that version.

Docs of newer versions have the same text, so I wonder how this could be working for you?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we'd need to create a new script for this that spins up the required docker containers, creates the dump, moves the file and destroys itself, similar to what we have in ci/update-packages.sh?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Might be interesting. Maybe using Docker pg containers for development makes more sense in general.

Do you propose this both for restore and dump?

I want to restore into my local database.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you propose this both for restore and dump?

Yes, but only in the context of dumping from and restoring to a (newer) remote database server. For restoring to an older local database server, I'm not sure whether going with the docker-route would help with anything.

@Flix6x Flix6x modified the milestones: 0.18.0, 0.19.0 Dec 20, 2023
…ant cluster"

This reverts commit ba4859a.

Signed-off-by: F.N. Claessen <felix@seita.nl>
@nhoening nhoening modified the milestones: 0.19.0, 0.20 Feb 18, 2024
@Flix6x Flix6x modified the milestones: 0.20.0, 0.20.1, 0.21.0 Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants