Re: relfilenode statistics

Hi,

On Tue, Sep 10, 2024 at 05:30:32AM +0000, Bertrand Drouvot wrote:
> Hi,
>
> On Thu, Sep 05, 2024 at 04:48:36AM +0000, Bertrand Drouvot wrote:
> > Please find attached a mandatory rebase.
> >
> > In passing, checking if based on the previous discussion (and given that we
> > don't have the relation OID when writing buffers out) you see another approach
> > that the one this patch is implementing?
>
> Attached v5, mandatory rebase due to recent changes in the stats area.

Attached v6, mandatory rebase due to b14e9ce7d5.

Note that 0001 is the same as the one proposed in [0] and needs to be applied
here to make the stats machinery working as expected with the relfile added in
the stats hash key (though it deserves its own dedicated thread as explained in [0]).

Don't look at 0001 and 0002 as I think we need more design discussion.

=== Sum up the feedback received up-thread

I re-read this thread and it appears that there is 3 main remarks:

R1: Andres did propose to add stuff like "n_dead_tup" (see [1]), to provide
even more benefits.

R2: Robert mentioned ([2]) that we need to decide between "sometimes I
don't know the relation OID so I want to use the relfilenumber
instead, without changing the user experience" and "some
of these stats actually properly pertain to the relfilenode rather
than the relation so I want to associate them with the right object
and that will affect how the user sees things".

R3: Michael had concerns about adding a new field (the relfile) in the hash key,
see [3].

=== My thoughts:

While my initial idea was that the relfilenode stats would deal only with I/O
activities it also looks like that it would be benficial to add sutff like
"n_dead_tup".

Then I think we should go with the "sometimes I don't know the relation OID
so I want to use the relfilenumber instead, without changing the user experience"
way.

Regarding the concern about adding a new field in the hash key, I think we can't
avoid that as we don't have the relation OID when writing buffers out.

=== Moving forward

I would go for trying to store everything that is "relation" related into the
relfilenode stats (that will then include n_dead_tup among other things) and
try to hide the distinction between relfilenode stats and relation stats from
the user.

Thoughts of moving forward that way?

[0]: https://www.postgresql.org/message-id/Zyb7RW1y9dVfO0UH%40ip-10-97-1-34.eu-west-3.compute.internal
[1]: https://www.postgresql.org/message-id/20240607033806.6gwgolihss72cj6r%40awork3.anarazel.de
[2]: https://www.postgresql.org/message-id/CA%2BTgmoZtwT6h%3DnyuQ1J9GNSrRyhf0fv7Ai6FzO%3DbH0C9Bf6tew%40mail.gmail.com
[3]: https://www.postgresql.org/message-id/Zo9j69GhexDpeV4k%40paquier.xyz

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com