❮

Removing GlobalVisTestNonRemovableHorizon

  • Jump to comment-1
    andres@anarazel.de2024-04-15T18:57:31+00:00
    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
    • Jump to comment-1
      robertmhaas@gmail.com2024-04-15T19:14:08+00:00
      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
      • Jump to comment-1
        andres@anarazel.de2024-04-17T19:41:42+00:00
        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
    • Jump to comment-1
      michael@paquier.xyz2024-04-15T22:33:20+00:00
      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
      • Jump to comment-1
        andres@anarazel.de2024-04-17T19:42:07+00:00
        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..