Applying XLR_INFO_MASK correctly when looking at WAL record information

Started by Michael Paquierover 9 years ago4 messageshackers
Jump to latest
#1Michael Paquier
michael@paquier.xyz

Hi all,

I have just been trapped by the fact that in some code paths we look
at the RMGR information of a WAL record (record->xl_info) but the
filter dedicated to it, ~XLR_INFO_MASK, is not applied. This is
harmful now, but this could lead to problems if in the future new
record-level flags, similar to XLR_SPECIAL_REL_UPDATE, are added.
Disclaimer: I am looking at a patch where a record-level flag makes
sense, still this looks like a separate problem.

What about the patch attached to make things more consistent?
Thanks,
--
Michael

Attachments:

wal-xlinfo.patchtext/x-diff; charset=US-ASCII; name=wal-xlinfo.patchDownload+8-4
#2Ashutosh Sharma
ashu.coek88@gmail.com
In reply to: Michael Paquier (#1)
Re: Applying XLR_INFO_MASK correctly when looking at WAL record information

Hi,

What about the patch attached to make things more consistent?

I have reviewed this patch. It does serve the purpose and looks sane
to me. I am marking it as ready for committer.

With Regards,
Ashutosh Sharma
EnterpriseDB: http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#1)
Re: Applying XLR_INFO_MASK correctly when looking at WAL record information

Michael Paquier <michael.paquier@gmail.com> writes:

I have just been trapped by the fact that in some code paths we look
at the RMGR information of a WAL record (record->xl_info) but the
filter dedicated to it, ~XLR_INFO_MASK, is not applied. This is
harmful now, but this could lead to problems if in the future new
record-level flags, similar to XLR_SPECIAL_REL_UPDATE, are added.
Disclaimer: I am looking at a patch where a record-level flag makes
sense, still this looks like a separate problem.

What about the patch attached to make things more consistent?

Grepping found a couple of additional places that needed the same
fix. Pushed with those additions.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#3)
Re: Applying XLR_INFO_MASK correctly when looking at WAL record information

On Sat, Nov 5, 2016 at 2:29 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Michael Paquier <michael.paquier@gmail.com> writes:

I have just been trapped by the fact that in some code paths we look
at the RMGR information of a WAL record (record->xl_info) but the
filter dedicated to it, ~XLR_INFO_MASK, is not applied. This is
harmful now, but this could lead to problems if in the future new
record-level flags, similar to XLR_SPECIAL_REL_UPDATE, are added.
Disclaimer: I am looking at a patch where a record-level flag makes
sense, still this looks like a separate problem.

What about the patch attached to make things more consistent?

Grepping found a couple of additional places that needed the same
fix.

Ah, where wasShutdown is assigned. I thought I grepped it as well,
thanks for double-checking.

Pushed with those additions.

Thanks.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers