Prev: Adding variables for segment_size, wal_segment_sizeand block sizes
Next: Explain XML patch v2
From: Zdenek Kotala on 4 Jul 2008 10:38
I performed review of merged patch from Robert Treat. At first point the patch
does not work (SunOS 5.11 snv_86 sun4u sparc SUNW,Sun-Fire-V240)

truncate ... ok
alter_table ... FAILED (test process exited with exit code 2)
sequence ... ok
polymorphism ... ok
rowtypes ... ok
returning ... ok
largeobject ... FAILED (test process exited with exit code 2)
xml ... ok
test stats ... FAILED (test process exited with exit code 2)
test tablespace ... FAILED (test process exited with exit code 2)


However I went through a code and I have following comments:

1) Naming convention:

- Some probes uses "*end", some "*done". It would be good to select one name.
- I prefer to use clog instead of slru in probes name. clog is widely known.
- It seems to me better to have checkpoint-clog..., checkpoint-subtrans
instead of clog-checkpoint.
- buffer-flush was originally dirty-buffer-write-start. I prefer Robert Lor's
naming.

2) storage read write probes

smgr-read*, smgr-writes probes are in md.c. I personally think it make more
sense to put them into smgr.c. Only advantage to have it in md.c is that actual
amount of bytes is possible to monitor.

3) query rewrite probe

There are several probes for query measurement but query rewrite phase is
missing. See analyze_and_rewrite or pg_parse_and_rewrite

4) autovacuum_start

Autovacuum_start probe is alone. I propose following probes for completeness:

proc-autovacuum-start
proc-autovacuum-stop
proc-bgwriter-start
proc-bgwriter-stop
proc-backend-start
proc-backend-stop
proc-master-start
proc-master-stop

5) Need explain of usage:

I have some doubts about following probes. Could you please explain usage of
them? example dtrace script is welcome

- all exec-* probes
- mark-dirty, local-mark-dirty


6) several comments about placement:

I published patch on http://reviewdemo.postgresql.org/r/25/. I added several
comments there.

7) SLRU/CLOG

SLRU probes could be return more info. For example if page was in buffer or if
physical write is not necessary and so on.


That's all for this moment

Zdenek
--
Zdenek Kotala Sun Microsystems
Prague, Czech Republic http://sun.com/postgresql


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Alvaro Herrera on 4 Jul 2008 11:05
Zdenek Kotala wrote:

> 1) Naming convention:
>
> - Some probes uses "*end", some "*done". It would be good to select one name.
> - I prefer to use clog instead of slru in probes name. clog is widely known.

But slru is also used in pg_subtrans and pg_multixact. Which maybe
says that we oughta have separate probes for these rather than a single
one in slru. Otherwise it's going to be difficult telling one from the
other, yes?


> Autovacuum_start probe is alone. I propose following probes for completeness:
>
> proc-autovacuum-start
> proc-autovacuum-stop
> proc-bgwriter-start
> proc-bgwriter-stop

Separate proc-autovacuum-worker-start and proc-autovacuum-launcher-start,
perhaps. Not that I see any usefulness in tracking autovacuum launcher
start and stop, but then if we're tracking bgwriter start and stop then
it makes the same sense.

> proc-master-start
> proc-master-stop

What's "master" here?


--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Tom Lane on 4 Jul 2008 12:22
Alvaro Herrera <alvherre(a)commandprompt.com> writes:
>> Autovacuum_start probe is alone. I propose following probes for completeness:
>>
>> proc-autovacuum-start
>> proc-autovacuum-stop
>> proc-bgwriter-start
>> proc-bgwriter-stop

> Separate proc-autovacuum-worker-start and proc-autovacuum-launcher-start,
> perhaps. Not that I see any usefulness in tracking autovacuum launcher
> start and stop, but then if we're tracking bgwriter start and stop then
> it makes the same sense.

I see no value in cluttering the system with useless probes. The worker
start/stop are the only ones here with any conceivable application IMHO.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Zdenek Kotala on 4 Jul 2008 13:19
Alvaro Herrera napsal(a):
> Zdenek Kotala wrote:
>
>> 1) Naming convention:
>>
>> - Some probes uses "*end", some "*done". It would be good to select one name.
>> - I prefer to use clog instead of slru in probes name. clog is widely known.
>
> But slru is also used in pg_subtrans and pg_multixact. Which maybe
> says that we oughta have separate probes for these rather than a single
> one in slru. Otherwise it's going to be difficult telling one from the
> other, yes?

Yeah, you are right, I missed that it is used in other part too. slru is OK

>
>> Autovacuum_start probe is alone. I propose following probes for completeness:
>>
>> proc-autovacuum-start
>> proc-autovacuum-stop
>> proc-bgwriter-start
>> proc-bgwriter-stop
>
> Separate proc-autovacuum-worker-start and proc-autovacuum-launcher-start,
> perhaps. Not that I see any usefulness in tracking autovacuum launcher
> start and stop, but then if we're tracking bgwriter start and stop then
> it makes the same sense.

The advantage to track start and stop of procese is that you can stop the
process in dtrace script at the beginning and for example attach debugger or for
example start counting number of writes per process and so on.

>> proc-master-start
>> proc-master-stop
>
> What's "master" here?

Main process - postmaster.


Zdenek

--
Zdenek Kotala Sun Microsystems
Prague, Czech Republic http://sun.com/postgresql


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Zdenek Kotala on 4 Jul 2008 13:26
Tom Lane napsal(a):
> Alvaro Herrera <alvherre(a)commandprompt.com> writes:
>>> Autovacuum_start probe is alone. I propose following probes for completeness:
>>>
>>> proc-autovacuum-start
>>> proc-autovacuum-stop
>>> proc-bgwriter-start
>>> proc-bgwriter-stop
>
>> Separate proc-autovacuum-worker-start and proc-autovacuum-launcher-start,
>> perhaps. Not that I see any usefulness in tracking autovacuum launcher
>> start and stop, but then if we're tracking bgwriter start and stop then
>> it makes the same sense.
>
> I see no value in cluttering the system with useless probes. The worker
> start/stop are the only ones here with any conceivable application IMHO.

As I answered to Alvaro. I needed to catch start of backend several times to
track call flow or attach debugger. It is possible to use some other dtrace
magic for that, but it is not easy and there is not way how to determine what
kind of process it is. For example how to measure how many writes performs
bgwriter?

Zdenek


--
Zdenek Kotala Sun Microsystems
Prague, Czech Republic http://sun.com/postgresql


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

 |  Next  |  Last
Pages: 1 2 3
Prev: Adding variables for segment_size, wal_segment_sizeand block sizes
Next: Explain XML patch v2