From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: jgdr@dalibo.com
Cc: andres@anarazel.de
Cc: michael@paquier.xyz
Cc: sawada.mshk@gmail.com
Cc: peter.eisentraut@2ndquadrant.com
Cc: pgsql-hackers@lists.postgresql.org
Cc: thomas.munro@enterprisedb.com
Cc: sk@zsrv.org
Cc: michael.paquier@gmail.com
Subject: Re: [HACKERS] Restricting maximum keep segments by repslots
Date: Tue, 31 Mar 2020 12:07:55 -0300
Message-ID: <20200331150755.GA3858@alvherre.pgsql> (raw)
In-Reply-To: <20200331.142016.971756479150607227.horikyota.ntt@gmail.com>
On 2020-Mar-31, Kyotaro Horiguchi wrote:
> Thank you for looking this and trouble rebasing!
>
> At Mon, 30 Mar 2020 20:03:27 -0300, Alvaro Herrera <alvherre@2ndquadrant.com> wrote in
> > I rebased this patch; it's failing to apply due to minor concurrent
> > changes in PostgresNode.pm. I squashed the patches in a series that
> > made the most sense to me.
> >
> > I have a question about static variable lastFoundOldestSeg in
> > FindOldestXLogFileSegNo. It may be set the first time the function
> > runs; if it is, the function never again does anything, it just returns
> > that value. In other words, the static value is never reset; it never
> > advances either. Isn't that strange? I think the coding is to assume
> > that XLogCtl->lastRemovedSegNo will always be set, so its code will
> > almost never run ... except when the very first wal file has not been
> > removed yet. This seems weird and pointless. Maybe we should think
> > about this differently -- example: if XLogGetLastRemovedSegno returns
> > zero, then the oldest file is the zeroth one. In what cases this is
> > wrong? Maybe we should fix those.
>
> That's right, but without the static variable, every call to the
> pg_replication_slots view before the fist checkpoint causes scanning
> pg_xlog. XLogCtl->lastRemovedSegNo advances only at a checkpoint, so
> it is actually right that the return value from
> FindOldestXLogFileSegNo doesn't change until the first checkpoint.
>
> Also we could set XLogCtl->lastRemovedSegNo at startup, but the
> scanning on pg_xlog is useless in most cases.
>
> I avoided to update the XLogCtl->lastRemovedSegNo directlry, but the
> third way would be if XLogGetLastRemovedSegno() returned 0, then set
> XLogCtl->lastRemovedSegNo by scanning the WAL directory. The attached
> takes this way.
I'm not sure if I explained my proposal clearly. What if
XLogGetLastRemovedSegno returning zero means that every segment is
valid? We don't need to scan pg_xlog at all.
> > Regarding the PostgresNode change in 0001, I think adding a special
> > parameter for primary_slot_name is limited. I'd like to change the
> > definition so that anything that you give as a parameter that's not one
> > of the recognized keywords (has_streaming, etc) is tested to see if it's
> > a GUC; and if it is, then put it in postgresql.conf. This would have to
> > apply both to PostgresNode::init() as well as
> > PostgresNode::init_from_backup(), obviously, since it would make no
> > sense for the APIs to diverge on this point. So you'd be able to do
> > $node->init_from_backup(allow_streaming => 1, work_mem => "4MB");
> > without having to add code to init_from_backup to handle work_mem
> > specifically. This could be done by having a Perl hash with all the GUC
> > names, that we could read lazily from "postmaster --describe-config" the
> > first time we see an unrecognized keyword as an option to init() /
> > init_from_backup().
>
> Done that way. We could exclude "known" parameters by explicitly
> delete the key at reading it, but I choosed to enumerate the known
> keywords. Although it can be used widely but actually I changed only
> 018_repslot_limit.pl to use the feature.
Hmm. I like this idea in general, but I'm not sure I want to introduce
it in this form right away. For the time being I realized while waking
up this morning we can just use $node->append_conf() in the
018_replslot_limit.pl file, like every other place that needs a special
config. There's no need to change the test infrastructure for this.
I'll go through this again. Many thanks,
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: alvherre@2ndquadrant.com, horikyota.ntt@gmail.com, jgdr@dalibo.com, andres@anarazel.de, michael@paquier.xyz, sawada.mshk@gmail.com, peter.eisentraut@2ndquadrant.com, pgsql-hackers@lists.postgresql.org, thomas.munro@enterprisedb.com, sk@zsrv.org, michael.paquier@gmail.com
Subject: Re: [HACKERS] Restricting maximum keep segments by repslots
In-Reply-To: <20200331150755.GA3858@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox