Commit/abort WAL records with dropped rels missing XLR_SPECIAL_REL_UPDATE
While hacking on pg_rewind, I noticed that commit and abort WAL records
are never marked with the XLR_SPECIAL_REL_UPDATE flag. But if the record
contains "dropped relfilenodes", surely it should be?
It's harmless as far as the backend and all the programs in PostgreSQL
repository are concerned, but the point of XLR_SPECIAL_REL_UPDATE is to
aid external tools that try to track which files are modified. Attached
is a patch to fix it.
It's always been like that, but I am not going backport, for fear of
breaking existing applications. If a program reads the WAL, and would
actually need to do something with commit records dropping relations,
that seems like such a common scenario that the author should've thought
about it and handled it even without the flag reminding about it. Fixing
it in master ought to be enough.
Thoughts?
- Heikki
Attachments:
0001-Make-xact.h-usable-in-frontend.patchtext/x-patch; charset=UTF-8; name=0001-Make-xact.h-usable-in-frontend.patchDownload+4-2
0002-Mark-commit-and-abort-WAL-records-with-XLR_SPECIAL_R.patchtext/x-patch; charset=UTF-8; name=0002-Mark-commit-and-abort-WAL-records-with-XLR_SPECIAL_R.patchDownload+29-2
On Fri, Aug 14, 2020 at 2:17 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
While hacking on pg_rewind, I noticed that commit and abort WAL records
are never marked with the XLR_SPECIAL_REL_UPDATE flag. But if the record
contains "dropped relfilenodes", surely it should be?
Right.
It's harmless as far as the backend and all the programs in PostgreSQL
repository are concerned, but the point of XLR_SPECIAL_REL_UPDATE is to
aid external tools that try to track which files are modified. Attached
is a patch to fix it.It's always been like that, but I am not going backport, for fear of
breaking existing applications. If a program reads the WAL, and would
actually need to do something with commit records dropping relations,
that seems like such a common scenario that the author should've thought
about it and handled it even without the flag reminding about it. Fixing
it in master ought to be enough.
+1 for doing it in master only. Even if someone comes up with such a
scenario for back-branches, we can revisit our decision to backport
this but like you, I also don't see any pressing need to do it now.
--
With Regards,
Amit Kapila.
On Sat, Aug 15, 2020 at 11:05:43AM +0530, Amit Kapila wrote:
On Fri, Aug 14, 2020 at 2:17 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
It's always been like that, but I am not going backport, for fear of
breaking existing applications. If a program reads the WAL, and would
actually need to do something with commit records dropping relations,
that seems like such a common scenario that the author should've thought
about it and handled it even without the flag reminding about it. Fixing
it in master ought to be enough.+1 for doing it in master only. Even if someone comes up with such a
scenario for back-branches, we can revisit our decision to backport
this but like you, I also don't see any pressing need to do it now.
+1.
--
MichaelOn 17/08/2020 10:00, Michael Paquier wrote:
On Sat, Aug 15, 2020 at 11:05:43AM +0530, Amit Kapila wrote:
On Fri, Aug 14, 2020 at 2:17 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
It's always been like that, but I am not going backport, for fear of
breaking existing applications. If a program reads the WAL, and would
actually need to do something with commit records dropping relations,
that seems like such a common scenario that the author should've thought
about it and handled it even without the flag reminding about it. Fixing
it in master ought to be enough.+1 for doing it in master only. Even if someone comes up with such a
scenario for back-branches, we can revisit our decision to backport
this but like you, I also don't see any pressing need to do it now.+1.
Pushed, thanks!
- Heikki