Обсуждение: Removing GlobalVisTestNonRemovableHorizon

Поиск
Список
Период
Сортировка

Removing GlobalVisTestNonRemovableHorizon

От
Andres Freund
Дата:
Hi,

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.

Greetings,

Andres Freund



Re: Removing GlobalVisTestNonRemovableHorizon

От
Robert Haas
Дата:
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



Re: Removing GlobalVisTestNonRemovableHorizon

От
Michael Paquier
Дата:
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

Вложения

Re: Removing GlobalVisTestNonRemovableHorizon

От
Andres Freund
Дата:
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



Re: Removing GlobalVisTestNonRemovableHorizon

От
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..