Re: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review] (Fix memory growth)

From: Amit Kapila <amit(dot)kapila(at)huawei(dot)com>
To: "'Greg Smith'" <greg(at)2ndQuadrant(dot)com>
Cc: "'Andres Freund'" <andres(at)2ndquadrant(dot)com>, "'Boszormenyi Zoltan'" <zb(at)cybertec(dot)at>, <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review] (Fix memory growth)
Date: 2013-03-15 10:57:29
Message-ID: 004801ce216b$e37c3b30$aa74b190$@kapila@huawei.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thursday, March 14, 2013 6:58 PM Amit Kapila wrote:
> On Monday, March 11, 2013 12:18 PM Amit Kapila wrote:
> > On Sunday, March 10, 2013 8:43 PM Greg Smith wrote:
> > > On 3/7/13 2:42 AM, Amit Kapila wrote:
> > > > I also think the tests added for regression may be more than
> > > required...
> > > > If you think above optimization's to reduce number of tests are
> > okay,
> > > then I
> > > > will update the patch.
> > > 7) If I run SET PERSISTENT a lot concurrently, something happens
> > > that slows down connecting to the server. Restarting the server
> > > makes it
> > go
> > > away. I have a pgbench test case demonstrating the problem below,
> > > in the "Concurrency" section. I haven't tried to replicate it on
> > another
> > > system yet.
> > >
> > > = Tested features that work fine =
> > >
> > > Entries added here are tracked as you'd expect:
> > >
> > > $ psql -c "set persistent checkpoint_segments='32'"
> > > $ pg_ctl reload
> > > $ psql -xc "select name,setting,sourcefile,sourceline from
> > pg_settings
> > > where name='checkpoint_segments'"
> > > name | checkpoint_segments
> > > setting | 32
> > > sourcefile |
> > > /Users/gsmith/pgwork/data/autoconf/config/postgresql.auto.conf
> > > sourceline | 1
> > >
> > > When the postgresql.auto.conf file is missing and you use SET
> > > PERSISTENT, it quietly creates it when writing out the new setting.
> > >
> > > = Concurrency and performance slowdown =
> > >
> > > I made two pgbench scripts that adjust a guc, one user and the
> other
> > > sighup, to a random value:
> > >
> > > random-join-limit.sql:
> > >
> > > \setrandom limit 1 8
> > > set persistent join_collapse_limit=:limit; select pg_reload_conf();
> > >
> > > random-segments.sql:
> > >
> > > \setrandom segments 3 16
> > > set persistent checkpoint_segments=:segments; select
> > > pg_reload_conf();
> > >
> > > I then fired off a bunch of these in parallel:
> > >
> > > pgbench -n -f random-join-limit.sql -f random-segments.sql -c 8 -T
> > > 60
> > >
> > > This ran at 1547 TPS on my 8 core Mac, so that's not bad. No
> > > assertions and the config file was still valid at the end, which is
> > > a good sign the locking method isn't allowing utter chaos. Without
> > > the
> > > pg_reload_conf()
> > > in the test files, it also completed. With the reload happening in
> > one
> > > file but not the other, things were also fine.
> > >
> > > However, one thing I saw is that the server got significantly
> slower
> > > the more I ran this test script. After a few minutes it was down
> to
> > > only 400 TPS. The delay shows up between when I run psql and when
> I
> > > get
> > the
> > > prompt back. Here's normal behavior:
> > >
> > > $ time psql -c "select 1"
> > > real 0m0.006s
> > >
> > > And here's what I get after a single run of this pgbench hammering:
> > >
> > > $ time psql -c "select 1"
> > > real 0m0.800s
> > >
> > > 800ms? The slowdown is all for psql to start and connect, it's not
> > in
> > > the executor:
>
> I suspect this is due to pg_reload_conf(). I am looking into it.
> However could you please try once without pg_reload_conf().

The memory growth is because of pg_reload_conf().
There were 2 sources of memory growth:
1. Load_ident - Memory context was not getting deleted
2. ProcessConfigFile - Most of the memory for parsing and processing is
allocated under TopMemoryContext and is not getting reset,
which leads to this problem. For now I have fixed by
doing this in separate context.

The above 2 fixes are in attached patch 'ctx_growth_fix_v1'.

Apart from this, I have fixed issues 1 and 2 reported by you in attached
version 'set_persistent_v13', by sending signal at end of command
and checking for parameters which cannot be loaded.

With this, all the issues reported by you are addressed.

Thanks to you for so careful test and review of this patch.

With Regards,
Amit Kapila.

Attachment Content-Type Size
ctx_growth_fix_v1.patch application/octet-stream 1.8 KB
set_persistent_v13.patch application/octet-stream 72.6 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Ants Aasma 2013-03-15 12:32:57 Re: Enabling Checksums
Previous Message Boszormenyi Zoltan 2013-03-15 09:20:47 Re: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]