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

support --wal-dir in keeper #865

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

sebasmannem
Copy link

Trying to get the --wal-dir option from #641 merged.

For those who run Postgres with a WAL directory symlinked to a separate
disk (often for performance reasons) this commit adds a --wal-dir flag
to the keeper.

The symlink to a new WAL disk needs to be created as part of the
database initialisation step, and also whenever we clobber the database,
such as when taking a pg_basebackup (if pg_rewind is
disabled/unavailable). By passing this directory as an argument to
initdb and pg_basebackup, we can cover all the scenarios that the keeper
erases and recreates the data directory, ensuring the symlink is always
present.

* Support waldir in pitr command

For those who use different devices for wal and data directory, we
should support the interpolation of the wal directory location in the
pitr command as we do the data directory. This allows people to supply
commands like:

```
pg_basebackup --pgdata %d --waldir %w ...
```

And have both data directory and wal directory templated. pg_basebackup
and initdb are commands that support the concept of waldir, but this
could be useful for any arbitrary pitr command.

The commit also simplifies the implementation of the expand function to
feature less mutation and slice offset accesses.

* Also remove wal directory when destroying data

If a wal directory is supplied then it's important to clean-out our wals
along with our data directory. This commit renames the existing
RemoveAll method to RemoveAllIfInitialized, which better represents what
it does, and has it rely on a RemoveAll method that cleans-up both data
directory and wal.

Co-authored-by: Harry Maclean <[email protected]>
@sebasmannem
Copy link
Author

@sgotti ,
In this first step I have just forked the code that was already there and rebased it on current master.
I think it looks very good by itself already, and the gocardless guys mentioned they run with this succesfully for a while now.
But I will compile a version myself and test some.
The only thing I can think of that we could add, would be that
if you have a datadir setup with local pg_wal, and the keeper is about to start postgres (restart, patch, whatever happens),
we could have stolon move the wal-dir just before it starts postgres.
I will have a look at that and something in there too.
Please let me know if there is anything else to fix before we merge...

@sebasmannem sebasmannem marked this pull request as ready for review February 21, 2022 11:44
@sebasmannem
Copy link
Author

@sgotti , I think this is ready, but the agola test doesn't look too good.
And I reran the tests, but they failed again.
Can you help me forward resolving this?
Like where should I look, what can I do to investigate deeper, etc...

@sebasmannem
Copy link
Author

@sgotti this is ready to be merged.
Can you have a look?

@sgotti
Copy link
Member

sgotti commented May 24, 2022

@sebasmannem Thanks. I'll take a look at this in the next days.

In the meantime there're some lint errors on go1.16. For go 1.15 you're using a package not available in such version but I'll bump the go versions to the last two major versions in another PR.

@sebasmannem
Copy link
Author

sebasmannem commented May 26, 2022 via email

@sebasmannem
Copy link
Author

@sgotti I intend to finalise this and my other commit over the next few weeks.
How is your bump PR going? Anything I can do to help?

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.

4 participants