On 2022-09-01 14:18:42 +0200, Matthias van de Meent wrote: > On Wed, 31 Aug 2022 at 20:56, Andres Freund <andres@anarazel.de> wrote: > > But given this is done when stats are flushed, which only happens after the > > transaction ended, we can just use GetCurrentTransactionStopTimestamp() - if > > we got to flushing the transaction stats we'll already have computed that. > > I'm not entirely happy with that, as that would still add function > call overhead, and potentially still call GetCurrentTimestamp() in > this somewhat hot loop.
We already used GetCurrentTransactionStopTimestamp() (as you reference below) before we get to this point, so I doubt that we'll ever call GetCurrentTimestamp(). And it's hard to imagine that the function call overhead of GetCurrentTransactionStopTimestamp() matters compared to acquiring locks etc.
Vik and I looked at this a little, and found that we actually don't have generally have GetCurrentTransactionStopTimestamp() at this point - a simple 'select * from pg_class' will result in 9 passes of this code, none of which have xactStopTimestamp != 0.
After discussing it a little, we came to the conclusion that for the stated use case, xactStartTimestamp is actually accurate enough, provided that we only ever update it with a newer value. It would only likely be in extreme edge-cases where the difference between start and end transaction time would have any bearing on whether or not one might drop a table/index for lack of use.
Doing it this way also means we no longer need the GUC to enable the feature, which as Bruce notes, is likely to lose 95% of users.
Updated patch attached:
- GUC removed.
- The timestamp recorded is xactStartTimestamp.
- Docs updated to make it clear we're recording transaction start time.