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

nixos/postgresql: improve docs on how to upgrade #149013

Merged
merged 1 commit into from
Dec 12, 2021

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Dec 6, 2021

Motivation for this change
  • It's IMHO a slight overkill to deploy an additional container even if
    it's never supposed to be running. Also, the currently suggested
    approach wouldn't use the default state-directory for the new version.
  • Explain the structure of the state-directories and where the
    version-numbers are actually coming from.
  • Mention ./analyze_new_cluster.sh & ./delete_old_cluster.sh.
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@Ma27 Ma27 requested a review from danbst December 6, 2021 13:41
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Dec 6, 2021
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10 labels Dec 6, 2021
@maralorn
Copy link
Member

maralorn commented Dec 9, 2021

Oh, nice!

Can you maybe make explicit in which directory to run the two scripts at the end?

I really wished we could make this still easier to deploy for users. e.g. have a pkg.postgres_12_to_13_migration to install? We can probably auto-generate those from the versions of the available postgres packages?

@Ma27
Copy link
Member Author

Ma27 commented Dec 10, 2021

Can you maybe make explicit in which directory to run the two scripts at the end?

Good idea, will do! :)

I really wished we could make this still easier to deploy for users. e.g. have a pkg.postgres_12_to_13_migration to install? We can probably auto-generate those from the versions of the available postgres packages?

Considering that this is a highly imperative step that can cause a lot of problems if done wrong, I'd like administrators to actually think twice about it before doing that.

Also, there's way too much stuff someone has to take care of on their own:

  • stopping - if needed - services writing to the database
  • which datadir to use (you can still override services.postgresql.dataDir)

All in all, I think we'd have to assume too much when implementing that.

@maralorn
Copy link
Member

Fair.

Also, there's way too much stuff someone has to take care of on their own:

* stopping - if needed - services writing to the database

* which datadir to use (you can still override `services.postgresql.dataDir`)

Those are good points, maybe also mention them in the explanation?^^

Not a must though.

* It's IMHO a slight overkill to deploy an additional container even if
  it's never supposed to be running. Also, the currently suggested
  approach wouldn't use the default state-directory for the new version.
* Explain the structure of the state-directories and where the
  version-numbers are actually coming from.
* Mention `./analyze_new_cluster.sh` & `./delete_old_cluster.sh`.
@Ma27
Copy link
Member Author

Ma27 commented Dec 11, 2021

@maralorn updated accordingly :)

@maralorn maralorn merged commit b243326 into NixOS:master Dec 12, 2021
@maralorn
Copy link
Member

Thx!

@Ma27 Ma27 deleted the postgres-docs branch December 12, 2021 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants