Обсуждение: Why is it OK for dbsize.c to look at relation files directly?

Hi,

To me it seems like a noticable layering violation for dbsize.c to
directly stat() files in the filesystem, without going through
smgr.c. And now tableam.

This means there's code knowing about file segments outside of md.c -
which imo shouldn't be the case.  We also stat a lot more than
necessary, if the relation is already open - when going through smgr.c
we'd only need to stat the last segment, rather than all previous
segments.

Always going through smgr would have the disadvantage that we'd probably
need a small bit of logic to close the relation's smgr references if it
previously wasn't open, to avoid increasing memory usage unnecessarily.


dbsize.c directly stat()ing files, also means that if somebody were to
write a tableam that doesn't store data in postgres compatible segmented
files, the size can't be displayed.


I think we should change dbsize.c to call
RelationGetNumberOfBlocksInFork() for relkinds != TABLE/TOAST/MATVIEW,
and a new AM callback for those. Probably with the aforementioned
additional logic of closing smgr references if they weren't open before
the size computation.

Imo this pretty clearly is v13 work.

I'd assume that pg_database_size*() would continue the same way it does
right now.

Greetings,

Andres Freund

On Wed, Apr 24, 2019 at 04:09:56PM -0700, Andres Freund wrote:
> I think we should change dbsize.c to call
> RelationGetNumberOfBlocksInFork() for relkinds != TABLE/TOAST/MATVIEW,
> and a new AM callback for those. Probably with the aforementioned
> additional logic of closing smgr references if they weren't open before
> the size computation.
>
> Imo this pretty clearly is v13 work.

Agreed that this is out of 12's scope.  Perhaps you should add a TODO
item for that?
--
Michael

On 2019-04-25 09:16:50 +0900, Michael Paquier wrote:
> Perhaps you should add a TODO item for that?

Just so it's guaranteed that it'll never happen? :)

On Wed, Apr 24, 2019 at 05:18:08PM -0700, Andres Freund wrote:
> Just so it's guaranteed that it'll never happen? :)

Items get done, from time to time...  b0eaa4c is one rare example :p

/me runs and hides.
--
Michael

On Thu, Apr 25, 2019 at 09:25:03AM +0900, Michael Paquier wrote:
>On Wed, Apr 24, 2019 at 05:18:08PM -0700, Andres Freund wrote:
>> Just so it's guaranteed that it'll never happen? :)
>
>Items get done, from time to time...  b0eaa4c is one rare example :p
>
>/me runs and hides.

IMO this is one of the rare examples of a TODO item that is actually
doable and not "I have no idea how to do this so I'll stick it into the
TODO list". And it seems like a fairly well isolated stuff, so it might
be a nice work for someone new.

regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services