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

From Amit Kapila
Subject Re: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review] (Fix memory growth)
Date
Msg-id 004801ce216b$e37c3b30$aa74b190$@kapila@huawei.com
Whole thread Raw
In response to Re: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]  (Amit Kapila <amit.kapila@huawei.com>)
List 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
havefixed 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.

pgsql-hackers by date:

Previous
From: Boszormenyi Zoltan
Date:
Subject: Re: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
Next
From: Ants Aasma
Date:
Subject: Re: Enabling Checksums