Remove XLogRecGetFullXid() in xlogreader.c?

Started by Michael Paquierabout 1 year ago4 messageshackers
Jump to latest
#1Michael Paquier
michael@paquier.xyz

Hi all,

XLogRecGetFullXid() has been introduced in 67b9b3ca3283 back in 2019,
but as far as I can see this has never been used in the code and this
is used nowhere in the core code.

I have looked at Debian's codesearch and also looked at traces of it
on github without seeing it being used anywhere. Knowing that this
was originally intended for a hypothetical undo log patch back then,
for which no work has been done for years, is there any point in
keeping this function in core?

This issue has been raised on a separate thread, where Noah has sent a
patch to consolidate a bit some epoch calculations for
FullTransactionIds, around here:
/messages/by-id/Z4i6MbUlZxjK1rRh@paquier.xyz

Removing it would have the benefit to do a bit less refactoring for
some of the work of the other thread, and this removes all traces of
-DFRONTEND in xlogreader.h. ;)

Thoughts or comments?
--
Michael

Attachments:

0001-Remove-XLogRecGetFullXid.patchtext/x-diff; charset=us-asciiDownload+1-43
#2Nathan Bossart
nathandbossart@gmail.com
In reply to: Michael Paquier (#1)
Re: Remove XLogRecGetFullXid() in xlogreader.c?

(I've added Thomas Munro to the thread.)

On Fri, Jan 17, 2025 at 02:00:49PM +0900, Michael Paquier wrote:

XLogRecGetFullXid() has been introduced in 67b9b3ca3283 back in 2019,
but as far as I can see this has never been used in the code and this
is used nowhere in the core code.

I have looked at Debian's codesearch and also looked at traces of it
on github without seeing it being used anywhere. Knowing that this
was originally intended for a hypothetical undo log patch back then,
for which no work has been done for years, is there any point in
keeping this function in core?

This issue has been raised on a separate thread, where Noah has sent a
patch to consolidate a bit some epoch calculations for
FullTransactionIds, around here:
/messages/by-id/Z4i6MbUlZxjK1rRh@paquier.xyz

Removing it would have the benefit to do a bit less refactoring for
some of the work of the other thread, and this removes all traces of
-DFRONTEND in xlogreader.h. ;)

Seems reasonable to me. I think the counterargument is that folks
developing new AMs should use this [0]/messages/by-id/CA+hUKG+mLmuDjMi6o1dxkKvGRL56Y2Rz+iXAcrZV03G9ZuFQ8Q@mail.gmail.com, but if no such users have
materialized in several years, then maybe that's no longer a concern.

[0]: /messages/by-id/CA+hUKG+mLmuDjMi6o1dxkKvGRL56Y2Rz+iXAcrZV03G9ZuFQ8Q@mail.gmail.com

--
nathan

#3Andres Freund
andres@anarazel.de
In reply to: Michael Paquier (#1)
Re: Remove XLogRecGetFullXid() in xlogreader.c?

Hi,

On 2025-01-17 14:00:49 +0900, Michael Paquier wrote:

XLogRecGetFullXid() has been introduced in 67b9b3ca3283 back in 2019,
but as far as I can see this has never been used in the code and this
is used nowhere in the core code.

I have looked at Debian's codesearch and also looked at traces of it
on github without seeing it being used anywhere. Knowing that this
was originally intended for a hypothetical undo log patch back then,
for which no work has been done for years, is there any point in
keeping this function in core?

This issue has been raised on a separate thread, where Noah has sent a
patch to consolidate a bit some epoch calculations for
FullTransactionIds, around here:
/messages/by-id/Z4i6MbUlZxjK1rRh@paquier.xyz

Removing it would have the benefit to do a bit less refactoring for
some of the work of the other thread, and this removes all traces of
-DFRONTEND in xlogreader.h. ;)

-0.5

I think it is sensible infrastructure and we'll eventually end up using
it. The cost of having it fairly low. If it were a legacy thing that we
shouldn't introduce new users for it'd be a different story, but it's the
opposite.

Greetings,

Andres

#4Michael Paquier
michael@paquier.xyz
In reply to: Andres Freund (#3)
Re: Remove XLogRecGetFullXid() in xlogreader.c?

On Fri, Jan 17, 2025 at 10:45:28AM -0500, Andres Freund wrote:

I think it is sensible infrastructure and we'll eventually end up using
it. The cost of having it fairly low. If it were a legacy thing that we
shouldn't introduce new users for it'd be a different story, but it's the
opposite.

The last 6 years are showing that we are still looking for such users,
so honestly, I am not sure if it's worth keeping. Let's see if others
have an opinion. I'm OK if the consensus is to let things be, as
well.
--
Michael