-
Notifications
You must be signed in to change notification settings - Fork 193
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
Check WAL shipping setup only in archiver mode #720
base: master
Are you sure you want to change the base?
Conversation
Setups with only streaming_archiver enabled will fail this check. This blocks "backup" operation, while receive-wal works. This appears to be previously reported issue, for example see EnterpriseDB#298 This bug may be explained by the fact previously enabling both streaming_archiver and archiver (WAL shipping) was the recommended configuration, per documentation. In such configuration this check would pass.
Hey, I'm glad to offer this tiny patch, but please take it very critically as it's my first time ever using barman (although not the first time setting up Postgres replication), so I might not know what I've done here. |
Hi @andrey-utkin - thanks for reporting the issue and providing a patch. The patch itself looks fine however skipping this particular check if The short explanation is that the Whether However when So there is still an opportunity here to improve the check when
|
- wals_directory must be writable - partial file should be present in case of streaming archiver
Hi, thanks a lot for the guidance! I've added the two checks you suggested. I have tested it on a new streaming-only setup, so it works at least on my machine. |
Kindly reminding to check the updated patch.
|
Hi @andrey-utkin - thanks for the reminder, we'll re-review this shortly. |
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.
Thanks for making the changes and sorry for making the suggestion about checking for .partial
files.
We still need to come up with a better way of verifying WAL streaming is working as intended in order to skip the xlog.db
check however we'll need to think about it a bit more to figure out the right approach.
hint="please make sure WAL shipping is setup", | ||
) | ||
if self.config.streaming_archiver: | ||
if not glob(os.path.join(self.config.streaming_wals_directory, |
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.
Unfortunately when I suggested this check I didn't take into account the fact that the .partial
file is not always going to be present. For example, when a WAL switch occurs there is no guarantee that the .partial
file for the next segment will have been created by the time the completed segment is renamed.
While there's still a need to improve on how Barman verifies WAL streaming is working, checking for the .partial
file is not the way to do it and we'll need to come up with something better.
if not os.access(self.config.wals_directory, os.W_OK): | ||
check_strategy.result( | ||
self.config.name, False, | ||
hint=f"wals_directory {self.config.wals_directory} must be writable" |
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.
This f
string should be fine for future Barman releases because they will require Python 3.6 as a minimum. If we did want to backport it to the 3.4.x branch then it would need to be compatible with Python 2.7 but that would be a small enough change to deal with if it came up.
Setups with only streaming_archiver enabled will fail this check. This blocks "backup" operation, while receive-wal works.
This appears to be previously reported issue, for example see #298
This bug may be explained by the fact previously enabling both streaming_archiver and archiver (WAL shipping) was the recommended configuration, per documentation. In such configuration this check would pass.