Re: Server instrumentation patch - Mailing list pgsql-hackers

> -----Original Message-----
> From: Bruce Momjian [mailto:pgman@candle.pha.pa.us]
> Sent: 24 June 2005 14:00
> To: Dave Page
> Cc: PostgreSQL-development; Andreas Pflug
> Subject: Re: Server instrumentation patch
>
> Well, I see Marc replying that dbsize should be moved completely from
> contrib:
>
>
> http://archives.postgresql.org/pgsql-patches/2005-06/msg00409.php
>
> The current version of the patch only moves those functions he wants.
> Marc says he wants them all moved, and I agree.

OK - did you see Andreas' response to why he hadn't done that (it was
actually posted in response to your original query, not Marcs)? In a
nutshell, the functions that had not been moved returned values that
were easily derived from the other functions, and thus could be
considered bloat?

The functions included in the patch were:

int8 pg_tablespace_size(oid)    - Return the size of the tablespace in
bytes
int8 pg_database_size(oid)    - Return the size of the database in
bytes
int8 pg_relation_size(oid)    - Return the size of the relation in
bytes
text pg_size_pretty(int8)    - Pretty-print the byte value

The ones left out were:

int8 database_size(name)    - Return the size of the database in
bytes (by name)
int8 relation_size(text)    - Return the size of the relation in
bytes (by name)
int8 indexes_size(text)       - Return the size of the indexes on the
named relation
int8 total_relation_size(text) - Return relation_size(text) +
indexes_size(text) + relation_size(text->toast tables)
setof record relation_size_components(text) - return a pretty-print set
of values from above.

I don't feel particularly strongly either way, but given the normal
desire not to bloat the backend necessarily, I have to question the need
to include the latter functions.

> > With the exception of the now removed pg_terminate_backend,
> I am unaware
> > of any issues that are outstanding. If the committers have
> issues they
> > *must* raise them for *any* submitted patch otherwise
> developers will
> > lose faith in the process when their hard work gets ignored.
>
> Well, from the May, 2005 discussion URL you posted, I see a
> clear reply
> to the idea of adding the I/O functions to the backend:
>
>
> http://archives.postgresql.org/pgsql-hackers/2005-05/msg00874.php
>
> Now, you can agree or disagree that there are issues with the I/O
> functions, but the concern was raised in May, and not
> addressed at all,
> either via email or the patch.

Maybe that's the wrong URL, but all I see there is a vague recollection
from Tom that there were security issues. In the next message, Andreas
recalls how you and he worked out the issues that were raised - I
believe this is the thread
http://archives.postgresql.org/pgsql-hackers/2004-07/msg00793.php.
Mhonarc has made a mess of the thread so it does seem to break in a few
places, and it is possible I've missed part.

> > Now, to try to get this ball rolling again - do the
> committers or anyone
> > else have any outstanding issues with the instrumentation or dbsize
> > patches that haven't been answered in public discussion and
> addressed in
> > the patches already?
>
> OK, agreed, how can we move forward?  The patch has three parts:
>
>     o  file I/O
>     o  move dbsize from contrib
>     o  backend terminate
>
> For the first, we need to re-discuss this on hackers.  I found this as
> the conclusion from July of 2004 as it relates to the I/O functions:
>
>
> http://archives.postgresql.org/pgsql-patches/2004-07/msg00561.php
>

I've just read through that thread, and the only mention of security
concerns I can spot is one where you say yourself that they are a
non-issue!!

What are the actual outstanding concerns with these functions?

> For the second, please supply a patch that moves _all_ of dbsize into
> the main server.  I think we have agreement on that.

If that remains the case having seen my comments above echoing Andreas'
concerns then sure, if that's what it takes to get them in, so be it.
Please confirm either way.

> For backend terminate, I agree with Tom that we have to get SIGTERM
> working, and then the function can just send a SIGTERM signal.  Unless
> it is working 100%, we will not add a terminate function to
> SQL.  I will
> post separately on this topic.

Agreed.

Regards, Dave.