Removing GlobalVisTestNonRemovableHorizon

On Mon, Apr 15, 2024 at 2:57 PM Andres Freund <andres@anarazel.de> wrote:

GlobalVisTestNonRemovableHorizon()/GlobalVisTestNonRemovableFullHorizon() only
existed for snapshot_too_old - but that was removed in f691f5b80a8. I'm
inclined to think we should remove those functions for 17. No new code should
use them.

+1 for removing whatever people shouldn't be using. I recently spent a
lot of time looking at this code and it's quite complicated and hard
to understand. It would of course have been nice to have done this
sooner, but I don't think waiting for next release cycle will make
anything better.

--
Robert Haas
EDB: http://www.enterprisedb.com

On Mon, Apr 15, 2024 at 11:57:20AM -0700, Andres Freund wrote:

GlobalVisTestNonRemovableHorizon()/GlobalVisTestNonRemovableFullHorizon() only
existed for snapshot_too_old - but that was removed in f691f5b80a8. I'm
inclined to think we should remove those functions for 17. No new code should
use them.

RMT hat on. Feel free to go ahead and clean up that now. No
objections from here as we don't want to take the risk of this stuff
getting more used in the wild.
--
Michael

Hi,

On 2024-04-15 15:13:51 -0400, Robert Haas wrote:

It would of course have been nice to have done this sooner, but I don't
think waiting for next release cycle will make anything better.

I don't really know how it could have been discovered sooner. We don't have
any infrastructure for finding code that's not used anymore. And even if we
had something finding symbols that aren't referenced within the backend, we
have lots of functions that are just used by extensions, which would thus
appear unused.

In my local build we have several hundred functions that are not used within
the backend, according to -Wl,--gc-sections,--print-gc-sections. A lot of that
is entirely expected stuff, like RegisterXactCallback(). But there are also
long-unused things like TransactionIdIsActive().

Greetings,

Andres Freund

On 2024-04-16 07:32:55 +0900, Michael Paquier wrote:

On Mon, Apr 15, 2024 at 11:57:20AM -0700, Andres Freund wrote:

GlobalVisTestNonRemovableHorizon()/GlobalVisTestNonRemovableFullHorizon() only
existed for snapshot_too_old - but that was removed in f691f5b80a8. I'm
inclined to think we should remove those functions for 17. No new code should
use them.

RMT hat on. Feel free to go ahead and clean up that now. No
objections from here as we don't want to take the risk of this stuff
getting more used in the wild.

Cool. Pushed the removal..