Re: Server instrumentation for 8.1

There's still a lengthy discussion going on whether it's a good idea to
add a forth way to read pgsql's schema (pg_* tables, pg_* views,
information_schema, did I miss one?), but I'd like to see helper
functions for issues *not* covered in the core package.

- dbsize has been in contrib for a long time, though it appears to me as
quite a basic functionality to find out about storage needs.

- The superuser only generic file functions in the admin package have
been posted for 8.0, but where (more or less ) silently dropped. These
functions allow pgadmin to display the server logs, as well as editing
pg_hba.conf and postgresql.conf without console access to
whatever-pgsql-is-running-on. I'd like to see this at least as contrib
module (the functions are probably safer than pl_sh).

Both these modules are bundled with the pgsql win32 installer, and are
installed by default. Both are supported by (at least) pgAdmin.

- There was a pg_kill_backend function in pre-8.0, but it was dropped
because "it's too dangerous". Incidentially, I've been in the situation
more than once where I needed to kill a backend process that was running
wild; alternatively, I'd have to shutdown the whole server. I had to do
this on the linux console with kill -9 (fortunately I did have access),
or using the win32 task manager (same). This appears even more error
prone to me than to point to the malicious process and kill it (through
pgadmin/pg_kill_backend)

- We don't have a profiling facility to tap an individual backend for a
limited period to find out what the client is doing there, so we need to
use log_statement for this (I'd like to work on profiling, but I didn't
find the time so far). Consequently, we have to deal with long logfiles,
containing much stuff we don't need. In the past, I found it to be very
helpful if a fresh logfile could be used (on a private installation,
stop/start server), that's why my logfile process implementation did
include a logfile rotation trigger functionality. Tom didn't need it, so
he dropped it. I'd opt for re-adding it again.

Yes yes I know, all of these can be done by a local administrator with
console access and an editor and cmd line tools, but there are indeed
people that do *not* have console access, or like to use decent tools....

Regards,
Andreas

On Wed, May 11, 2005 at 04:44:21PM +0000, Andreas Pflug wrote:
>
> Yes yes I know, all of these can be done by a local administrator with
> console access and an editor and cmd line tools, but there are indeed
> people that do *not* have console access, or like to use decent tools....

Is there a reason they couldn't be bundled into a separate package,
and either put in contrib/ or (my preference) put on gforge or
whatever? The less-is-more approach in the default source seems to
me to be a good thing. I'm not convinced that packaged systems
should ship that way -- maybe these should be included in desktop
systems -- but enabled-by-default for many of these things seems to
me to be too dangerous.

A

--
Andrew Sullivan | ajs(at)crankycanuck(dot)ca
I remember when computers were frustrating because they *did* exactly what
you told them to. That actually seems sort of quaint now.
--J.D. Baldwin

On Wed, May 11, 2005 at 04:44:21PM +0000, Andreas Pflug wrote:
> There's still a lengthy discussion going on whether it's a good idea to
> add a forth way to read pgsql's schema (pg_* tables, pg_* views,
> information_schema, did I miss one?), but I'd like to see helper
> functions for issues *not* covered in the core package.
>
> - dbsize has been in contrib for a long time, though it appears to me as
> quite a basic functionality to find out about storage needs.

FWIW, I believe the new system views cover all the dbsize cases.
--
Jim C. Nasby, Database Consultant decibel(at)decibel(dot)org
Give your computer some brain candy! www.distributed.net Team #1828

Windows: "Where do you want to go today?"
Linux: "Where do you want to go tomorrow?"
FreeBSD: "Are you guys coming, or what?"

Andreas,

I think you bring up some good points, but I also think that each package you
propose needs to be dealt with individually.

> - dbsize has been in contrib for a long time, though it appears to me as
> quite a basic functionality to find out about storage needs.

Although not needed so much if the new system views are approved; we have a
view that calculates database size. Not that multiple options isn't cool.

> - The superuser only generic file functions in the admin package have
> been posted for 8.0, but where (more or less ) silently dropped. These
> functions allow pgadmin to display the server logs, as well as editing
> pg_hba.conf and postgresql.conf without console access to
> whatever-pgsql-is-running-on. I'd like to see this at least as contrib
> module (the functions are probably safer than pl_sh).

Heck, I didn't even see these. I was going to write some in pgperl for my own
use. These seem potentially very dangerous though, so we wouldn't want them
installed by default.

> - There was a pg_kill_backend function in pre-8.0, but it was dropped
> because "it's too dangerous". Incidentially, I've been in the situation
> more than once where I needed to kill a backend process that was running
> wild; alternatively, I'd have to shutdown the whole server. I had to do
> this on the linux console with kill -9 (fortunately I did have access),
> or using the win32 task manager (same). This appears even more error
> prone to me than to point to the malicious process and kill it (through
> pgadmin/pg_kill_backend)

Certainly. But this was dropped because Tom couldn't get the bugs out (as I
recall) and make it "safe" to use, even for the superuser. You could take a
stab at fixing it. Also, if this were an "enable-at-build-time" option, it
would also help defuse the security argument, since it wouldn't necessarily
be installed.

> - We don't have a profiling facility to tap an individual backend for a
> limited period to find out what the client is doing there, so we need to
> use log_statement for this (I'd like to work on profiling, but I didn't
> find the time so far). Consequently, we have to deal with long logfiles,
> containing much stuff we don't need. In the past, I found it to be very
> helpful if a fresh logfile could be used (on a private installation,
> stop/start server), that's why my logfile process implementation did
> include a logfile rotation trigger functionality. Tom didn't need it, so
> he dropped it. I'd opt for re-adding it again.

+1

> Yes yes I know, all of these can be done by a local administrator with
> console access and an editor and cmd line tools, but there are indeed
> people that do *not* have console access, or like to use decent tools....

To support Andrew's assertion, automated server room administration tools
(like Hyperic and Embarcadero) could really use the above tools.

--
--Josh

Josh Berkus
Aglio Database Solutions
San Francisco

On 2005-05-11, "Jim C. Nasby" <decibel(at)decibel(dot)org> wrote:
> On Wed, May 11, 2005 at 04:44:21PM +0000, Andreas Pflug wrote:
>> There's still a lengthy discussion going on whether it's a good idea to
>> add a forth way to read pgsql's schema (pg_* tables, pg_* views,
>> information_schema, did I miss one?), but I'd like to see helper
>> functions for issues *not* covered in the core package.
>>
>> - dbsize has been in contrib for a long time, though it appears to me as
>> quite a basic functionality to find out about storage needs.
>
> FWIW, I believe the new system views cover all the dbsize cases.

dbsize looks at the actual size of files on disk; newsysviews does not,
it shows estimated sizes as taken from relpages. newsysviews doesn't
allow you to see size info for databases you're not connected to.

However, newsysviews knows about toast tables, and gives a breakdown of
table size by main storage, toast and index.

--
Andrew, Supernews
http://www.supernews.com - individual and corporate NNTP services

Josh Berkus <josh(at)agliodbs(dot)com> writes:
>> - The superuser only generic file functions in the admin package have
>> been posted for 8.0, but where (more or less ) silently dropped. These
>> functions allow pgadmin to display the server logs, as well as editing
>> pg_hba.conf and postgresql.conf without console access to
>> whatever-pgsql-is-running-on. I'd like to see this at least as contrib
>> module (the functions are probably safer than pl_sh).

> Heck, I didn't even see these. I was going to write some in pgperl for my own
> use. These seem potentially very dangerous though, so we wouldn't want them
> installed by default.

My recollection is that they weren't "silently dropped", they were
explicitly rejected after much discussion because of security worries
(and possibly other concerns, I don't recall for sure anymore).

regards, tom lane

Josh Berkus wrote:

>>- dbsize has been in contrib for a long time, though it appears to me as
>>quite a basic functionality to find out about storage needs.
>
>
> Although not needed so much if the new system views are approved; we have a
> view that calculates database size.

First, as some other msg states the views will estimate the sizes,
dbsize uses actual file sizes. Second, in contrast to CKL, I would *not*
use these fancy new system views, because they mean yet another
dependency for pgAdmin.

> Not that multiple options isn't cool.

What do you mean? Those views aren't an option for me.

>
>>- The superuser only generic file functions in the admin package have
>>been posted for 8.0, but where (more or less ) silently dropped. These
>>functions allow pgadmin to display the server logs, as well as editing
>>pg_hba.conf and postgresql.conf without console access to
>>whatever-pgsql-is-running-on. I'd like to see this at least as contrib
>>module (the functions are probably safer than pl_sh).
>
>
> Heck, I didn't even see these. I was going to write some in pgperl for my own
> use. These seem potentially very dangerous though, so we wouldn't want them
> installed by default.

Not more dangerous than "drop table pg_class". The file functions are
restricted to the cluster directory.

>>- There was a pg_kill_backend function in pre-8.0, but it was dropped
>>because "it's too dangerous". Incidentially, I've been in the situation
>>more than once where I needed to kill a backend process that was running
>>wild; alternatively, I'd have to shutdown the whole server. I had to do
>>this on the linux console with kill -9 (fortunately I did have access),
>>or using the win32 task manager (same). This appears even more error
>>prone to me than to point to the malicious process and kill it (through
>>pgadmin/pg_kill_backend)
>
>
> Certainly. But this was dropped because Tom couldn't get the bugs out (as I
> recall) and make it "safe" to use, even for the superuser.

Hm, don't remember that, AFAIR it was removed after a discussion between
Tom and Dave that kill -9 would stress a code path that still isn't
elaborated too much. But in the situations mentioned above, I took the
risk and would like a function for that.

Regards,
Andreas

Tom Lane wrote:
> Josh Berkus <josh(at)agliodbs(dot)com> writes:
>
>>>- The superuser only generic file functions in the admin package have
>>>been posted for 8.0, but where (more or less ) silently dropped. These
>>>functions allow pgadmin to display the server logs, as well as editing
>>>pg_hba.conf and postgresql.conf without console access to
>>>whatever-pgsql-is-running-on. I'd like to see this at least as contrib
>>>module (the functions are probably safer than pl_sh).
>
>
>>Heck, I didn't even see these. I was going to write some in pgperl for my own
>>use. These seem potentially very dangerous though, so we wouldn't want them
>>installed by default.
>
>
> My recollection is that they weren't "silently dropped", they were
> explicitly rejected after much discussion because of security worries

These functions were thoroughly discussed between Bruce, you and me, and
I fixed all issues that came up until a point where Bruce would agree
the functions to be reasonably safe. But it condensed down to something
like "Tom doesn't like it" at the end. The functions are not only
superuser only, but also restricted to the db cluster directory, making
them no more dangerous than "drop table pg_class".

Regards,
Andreas

Andrew - Supernews wrote:

>
> dbsize looks at the actual size of files on disk; newsysviews does not,
> it shows estimated sizes as taken from relpages.

Which shows *net* size only, not actual size because non-vacuumed rows
are not covered. It is correct after a vacuum full only.

> newsysviews doesn't
> allow you to see size info for databases you're not connected to.
>
> However, newsysviews knows about toast tables, and gives a breakdown of
> table size by main storage, toast and index.

pg_relation_size allows checking of file sizes for these as well, and
pgAdmin will show toast and index sizes.

Regards,
Andreas

On 2005-05-12, Andreas Pflug <pgadmin(at)pse-consulting(dot)de> wrote:
>> These seem potentially very dangerous though, so we wouldn't want them
>> installed by default.
>
> Not more dangerous than "drop table pg_class".

Have you ever tried that?

test=# drop table pg_class;
ERROR: permission denied: "pg_class" is a system catalog

That said, there are several functions built-in already that allow the
superuser to write files (COPY TO and lo_export being the main ones).

>>>- There was a pg_kill_backend function in pre-8.0, but it was dropped
>>>because "it's too dangerous". Incidentially, I've been in the situation
>>>more than once where I needed to kill a backend process that was running
>>>wild; alternatively, I'd have to shutdown the whole server. I had to do
>>>this on the linux console with kill -9 (fortunately I did have access),
>>>or using the win32 task manager (same). This appears even more error
>>>prone to me than to point to the malicious process and kill it (through
>>>pgadmin/pg_kill_backend)
>>
>> Certainly. But this was dropped because Tom couldn't get the bugs out
>> (as I recall) and make it "safe" to use, even for the superuser.
>
> Hm, don't remember that, AFAIR it was removed after a discussion between
> Tom and Dave that kill -9 would stress a code path that still isn't
> elaborated too much. But in the situations mentioned above, I took the
> risk and would like a function for that.

There seems to be some confusion here - kill -9 on a single backend is
unlikely to ever be safe, since the backend could be in the middle of
updates to shared memory at the time, and while it is certainly possible
to code it so that cleanup can still be done in this case, the current
code certainly does not try. (I've done it for a database I wrote myself
in the past: what's needed to make it feasible is that all shared memory
updates, without exception, must either be atomic replacements of values
of type volatile sig_atomic_t or must be updates to data structures that
are already known to be in-use by the specific backend and which can be
cleaned up even if they are in an internally corrupt state. This is
unlikely to be either possible or desirable for postgres.)

What currently happens is that backends respond to kill -15 (_NOT_ -9)
by cleaning up and exiting. This code path is used for implementing the
stop -mfast option, which means that as it currently exists, the cleanup
only has to be good enough to let other backends get out of critical
sections and complete their own rollback-and-exit safely.

(I've never tried it, but I'd expect the postmaster to respond to a
kill -9 of an individual backend by doing a panic shutdown and restart
anyway, so there's no advantage to doing that rather than doing, say,
restart -mfast or -mimmediate. I _have_ on very rare occasions done
kill -15 on individual backends and got away with it, usually when those
backends were idle anyway, but I'd never recommend it as a routine
technique without a lot more confidence in the correctness of the code
in question.)

--
Andrew, Supernews
http://www.supernews.com - individual and corporate NNTP services

On 2005-05-12, Andreas Pflug <pgadmin(at)pse-consulting(dot)de> wrote:
> Andrew - Supernews wrote:
>> dbsize looks at the actual size of files on disk; newsysviews does not,
>> it shows estimated sizes as taken from relpages.
>
> Which shows *net* size only, not actual size because non-vacuumed rows
> are not covered. It is correct after a vacuum full only.

nonsense

relpages is updated from the value of RelationGetNumberOfBlocks(rel) which
is definitive (it gets the value from smgr which gets it from the physical
file sizes); the only inaccuracy is that it is correct only as of the time
that the pg_class row was last updated (as done by any VACUUM, any ANALYZE,
any CLUSTER or any CREATE INDEX on the table, at minimum).

--
Andrew, Supernews
http://www.supernews.com - individual and corporate NNTP services

Andrew - Supernews <andrew+nonews(at)supernews(dot)com> writes:
> What currently happens is that backends respond to kill -15 (_NOT_ -9)
> by cleaning up and exiting. This code path is used for implementing the
> stop -mfast option, which means that as it currently exists, the cleanup
> only has to be good enough to let other backends get out of critical
> sections and complete their own rollback-and-exit safely.

Exactly. In theory it probably works fine to allow one backend to exit
via kill -TERM, but it cannot be claimed that that behavior has been
tested to any significant extent --- "fast" shutdown is not stressing it
in the same way.

I think this is largely a question of someone doing a significant amount
of stress testing: gun live server processes with "kill -TERM" in an
active system, and keep an eye out for resource leaks, held locks, and
so on. It would be more convincing if the processes getting zapped are
executing a wide variety of SQL, too --- I'd not feel very confident
given only tests of killing, say, pgbench threads.

regards, tom lane

Andreas,

> First, as some other msg states the views will estimate the sizes,
> dbsize uses actual file sizes. Second, in contrast to CKL, I would *not*
> use these fancy new system views, because they mean yet another
> dependency for pgAdmin.

<grin> I like that. You're in favor of including the server tools because
you, personally, use them. You're against including the new system views
because you, personally, won't use them. Do I misunderstand you, or are you
failing to put things in a broader context?

--
Josh Berkus
Aglio Database Solutions
San Francisco

On Thursday 12 May 2005 10:24, Tom Lane wrote:
> Andrew - Supernews <andrew+nonews(at)supernews(dot)com> writes:
> > What currently happens is that backends respond to kill -15 (_NOT_ -9)
> > by cleaning up and exiting. This code path is used for implementing the
> > stop -mfast option, which means that as it currently exists, the cleanup
> > only has to be good enough to let other backends get out of critical
> > sections and complete their own rollback-and-exit safely.
>
> Exactly. In theory it probably works fine to allow one backend to exit
> via kill -TERM, but it cannot be claimed that that behavior has been
> tested to any significant extent --- "fast" shutdown is not stressing it
> in the same way.
>
> I think this is largely a question of someone doing a significant amount
> of stress testing: gun live server processes with "kill -TERM" in an
> active system, and keep an eye out for resource leaks, held locks, and
> so on. It would be more convincing if the processes getting zapped are
> executing a wide variety of SQL, too --- I'd not feel very confident
> given only tests of killing, say, pgbench threads.
>

Cause I know you wont be satisfied with anecdotal evidence, I thought I would
just say that I have done kill's on specific backends in a high load OLTP
process, with 1000+ active connections, for years and not had a problem with
it yet.

Not that I wouldn't like to see some specific, thorough testing on the matter,
but I'm perfectly comfortable with the previously provided function.

--
Robert Treat
Build A Brighter Lamp :: Linux Apache {middleware} PostgreSQL

On Thursday 12 May 2005 13:04, Josh Berkus wrote:
> Andreas,
>
> > First, as some other msg states the views will estimate the sizes,
> > dbsize uses actual file sizes. Second, in contrast to CKL, I would *not*
> > use these fancy new system views, because they mean yet another
> > dependency for pgAdmin.
>
> <grin> I like that. You're in favor of including the server tools because
> you, personally, use them. You're against including the new system views
> because you, personally, won't use them. Do I misunderstand you, or are
> you failing to put things in a broader context?

I think your mischarectorizing his position. It's more like because he can
see a valid use case with the server tools, but doesn't see a valid use case
for the newsysviews. Inciedentally I don't even know that he is really
against the newsysviews, but given that one of the use cases was admin tools,
and he has some experience in that area, he felt the need to debunk that
reasoning. Personally I agree with that portion... if were putting them in
for admin tools, we don't need them.

--
Robert Treat
Build A Brighter Lamp :: Linux Apache {middleware} PostgreSQL

Andrew - Supernews wrote:
> On 2005-05-12, Andreas Pflug <pgadmin(at)pse-consulting(dot)de> wrote:

>
> relpages is updated from the value of RelationGetNumberOfBlocks(rel) which
> is definitive (it gets the value from smgr which gets it from the physical
> file sizes); the only inaccuracy is that it is correct only as of the time
> that the pg_class row was last updated (as done by any VACUUM, any ANALYZE,
> any CLUSTER or any CREATE INDEX on the table, at minimum).

"accurate unless vacuum not running".
... which renders it quite useless to find out about *real* disk usage.
Interesting for the query planner, not for the admin to know when a
vacuum full might be necessary.

Regards,
Andreas