Thread: 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
Attachment
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
Attachment
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