Hi,
thanks for the review. I'll look into that in ~2 weeks, once the
pgconf.eu
is over. A few comments in the text below.
Dne 17.10.2012 12:34, Shigeru HANADA napsal:
Performance test
================
I tested 1000 tables case (each is copy of pgbench_branches with
100000
rows) on 1GB shared_buffers server. Please note that I tested on
MacBook air, i.e. storage is not HDD but SSD. Here is the test
procedure:
1) loop 1000 times
1-1) create copy table of pgbench_accounts as accounts$i
1-2) load 100000 rows
1-3) add primary key
1-4) select all rows to cache pages in shared buffer
2) BEGIN
3) loop 1000 times
3-1) DROP TABLE accounts$i
4) COMMIT
I don't think the 'load rows' and 'select all rows' is really
necessary.
And AFAIK sequential scans use small circular buffer not to pollute
shared
buffers, so I'd guess the pages are not cached in shared buffers
anyway.
Have you verified that, e.g. by pg_buffercache?
The numbers below are average of 5 trials.
---------+----------+-------------
Build | DROP * | COMMIT
---------+----------+-------------
Master | 0.239 ms | 1220.421 ms
Patched | 0.240 ms | 432.209 ms
---------+----------+-------------
* time elapsed for one DROP TABLE statement
IIUC the patch's target is improving COMMIT performance by avoiding
repeated buffer search loop, so this results show that the patch
obtained its goal.
Coding review
=============
I have some comments about coding.
* Some cosmetic changes are necessary.
* Variable j in DropRelFileNodeAllBuffersList seems redundant.
* RelFileNodeBackendIsTemp() macro is provided for checking whether
the
relation is local, so using it would be better.
Please see attached patch for changes above.
* As Robert commented, this patch adds DropRelFileNodeAllBuffersList
by
copying code from DropRelFileNodeAllBuffers. Please refactor it to
avoid code duplication.
* In smgrDoPendingDeletes, you free srels explicitly. Can't we leave
them to memory context stuff? Even it is required, at least pfree
must
be called in the case nrels == 0 too.
* In smgrDoPendingDeletes, the buffer srels is expanded in every
iteration. This seems a bit inefficient. How about doubling the
capacity when used it up? This requires additional variable, but
reduces repalloc call considerably.
* Just an idea, but if we can remove entries for local relations from
rnodes array before buffer loop in DropRelFileNodeAllBuffersList,
following bsearch might be more efficient, though dropping many
temporary tables might be rare.
Yes, I plan to do all of this.
Our system creates a lot of "working tables" (even 100.000) and we
need
to perform garbage collection (dropping obsolete tables) regularly.
This
often took ~ 1 hour, because we're using big AWS instances with lots
of
RAM (which tends to be slower than RAM on bare hw). After applying
this
patch and dropping tables in groups of 100, the gc runs in less than
4
minutes (i.e. a 15x speed-up).
Hm, my environment seems very different from yours. Could you show
the
setting of shared_buffers in your environment? I'd like to make my
test
environment as similar as possible to yours.
We're using m2.4xlarge instances (70G of RAM) with 10GB shared buffers.
Tomas
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
