Thread: [HACKERS] segment size depending *_wal_size defaults (was increasing thedefault WAL segment size)

Hi,

intentionally breaking the thread here, I want this one point to get
a bit wider audience.

The excerpt of the relevant discussion is:

On 2017-08-23 12:13:15 +0530, Beena Emerson wrote:
> >> +     /* set default max_wal_size and min_wal_size */
> >> +     snprintf(repltok, sizeof(repltok), "min_wal_size = %s",
> >> +                      pretty_wal_size(DEFAULT_MIN_WAL_SEGS));
> >> +     conflines = replace_token(conflines, "#min_wal_size = 80MB", repltok);
> >> +
> >> +     snprintf(repltok, sizeof(repltok), "max_wal_size = %s",
> >> +                      pretty_wal_size(DEFAULT_MAX_WAL_SEGS));
> >> +     conflines = replace_token(conflines, "#max_wal_size = 1GB", repltok);
> >> +
> >
> > Hm. So postgresql.conf.sample values are now going to contain misleading
> > information for clusters with non-default segment sizes.
> >
> > Have we discussed instead defaulting min_wal_size/max_wal_size to a
> > constant amount of megabytes and rounding up when it doesn't work for
> > a particular segment size?
> 
> This was not discussed.
> 
> In the original code, the min_wal_size and max_wal_size are computed
> in the guc.c for any wal_segment_size set at configure.
> 
>     {
>         {"min_wal_size", PGC_SIGHUP, WAL_CHECKPOINTS,
>             gettext_noop("Sets the minimum size to shrink the WAL to."),
>             NULL,
>             GUC_UNIT_MB
>         },
>         &min_wal_size_mb,
>         5 * (XLOG_SEG_SIZE / (1024 * 1024)), 2, MAX_KILOBYTES,
>         NULL, NULL, NULL
>     },
> 
>     {
>         {"max_wal_size", PGC_SIGHUP, WAL_CHECKPOINTS,
>             gettext_noop("Sets the WAL size that triggers a checkpoint."),
>             NULL,
>             GUC_UNIT_MB
>         },
>         &max_wal_size_mb,
>         64 * (XLOG_SEG_SIZE / (1024 * 1024)), 2, MAX_KILOBYTES,
>         NULL, assign_max_wal_size, NULL
>     },
> 
> Hence I have retained the same calculation for min_wal_size and
> max_wal_size. If we get consensus for fixing a default and updating
> when required, then I will change the code accordingly.

So the question is whether we want {max,min}_wal_size be sized in
multiples of segment sizes or as a proper byte size.  I'm leaning
towards the latter.

- Andres

On Wed, Aug 30, 2017 at 9:36 AM, Andres Freund <andres@anarazel.de> wrote:
> So the question is whether we want {max,min}_wal_size be sized in
> multiples of segment sizes or as a proper byte size.  I'm leaning
> towards the latter.

Logically in the code it is just a matter of adjusting multipliers. Do
you think that we should worry about wal segment sizes higher than
2GB? Support for int64 GUCs is not here yet.
-- 
Michael

On 2017-08-30 09:49:14 +0900, Michael Paquier wrote:
> On Wed, Aug 30, 2017 at 9:36 AM, Andres Freund <andres@anarazel.de> wrote:
> > So the question is whether we want {max,min}_wal_size be sized in
> > multiples of segment sizes or as a proper byte size.  I'm leaning
> > towards the latter.
> 
> Logically in the code it is just a matter of adjusting multipliers.

No code difficulties here, I think we just need to decide what we want.


> Do you think that we should worry about wal segment sizes higher than
> 2GB? Support for int64 GUCs is not here yet.

1GB will be the limit anyway.


Greetings,

Andres Freund

On Wed, Aug 30, 2017 at 10:06 AM, Andres Freund <andres@anarazel.de> wrote:
> On 2017-08-30 09:49:14 +0900, Michael Paquier wrote:
>> Do you think that we should worry about wal segment sizes higher than
>> 2GB? Support for int64 GUCs is not here yet.
>
> 1GB will be the limit anyway.

Yeah, but imagine that we'd want to raise that even more up. By using
bytes, int64 GUCs become mandatory, or we'd come back into using MBs
for this parameter, bringing us back to the original implementation.
That's something worth thinking about.
-- 
Michael

On 2017-08-30 10:14:22 +0900, Michael Paquier wrote:
> On Wed, Aug 30, 2017 at 10:06 AM, Andres Freund <andres@anarazel.de> wrote:
> > On 2017-08-30 09:49:14 +0900, Michael Paquier wrote:
> >> Do you think that we should worry about wal segment sizes higher than
> >> 2GB? Support for int64 GUCs is not here yet.
> >
> > 1GB will be the limit anyway.
> 
> Yeah, but imagine that we'd want to raise that even more up.

I'm doubtfull of that. But even if, it'd not be hard to GUC support.

- Andres

On 8/29/17 20:36, Andres Freund wrote:
> So the question is whether we want {max,min}_wal_size be sized in
> multiples of segment sizes or as a proper byte size.  I'm leaning
> towards the latter.

I'm not sure what the question is or what its impact would be.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Wed, Aug 30, 2017 at 11:20 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
> On 8/29/17 20:36, Andres Freund wrote:
>> So the question is whether we want {max,min}_wal_size be sized in
>> multiples of segment sizes or as a proper byte size.  I'm leaning
>> towards the latter.
>
> I'm not sure what the question is or what its impact would be.

FWIW, I get the question as: do we want the in-memory values of
min/max_wal_size to be calculated in MB (which is now the case) or
just bytes. Andres tends for using bytes.
-- 
Michael

On 2017-08-30 12:52:26 +0900, Michael Paquier wrote:
> On Wed, Aug 30, 2017 at 11:20 AM, Peter Eisentraut
> <peter.eisentraut@2ndquadrant.com> wrote:
> > On 8/29/17 20:36, Andres Freund wrote:
> >> So the question is whether we want {max,min}_wal_size be sized in
> >> multiples of segment sizes or as a proper byte size.  I'm leaning
> >> towards the latter.
> >
> > I'm not sure what the question is or what its impact would be.
> 
> FWIW, I get the question as: do we want the in-memory values of
> min/max_wal_size to be calculated in MB (which is now the case) or
> just bytes. Andres tends for using bytes.

Not quite.  There's essentially two things:

1) Currently the default for {min,max}_wal_size depends on the segment  size. Given that the segment size is about to
beconfigurable, that  seems confusing.
 
2) Currently wal_segment_size is measured in GUC_UNIT_XBLOCKS, which  requires us to keep two copies of the underlying
variable,one in  XBLOCKS one in bytes. I'd rather just have the byte variant.
 

Regards,

Andres

On 8/30/17 00:45, Andres Freund wrote:
> 1) Currently the default for {min,max}_wal_size depends on the segment
>    size. Given that the segment size is about to be configurable, that
>    seems confusing.

On the one hand, I agree that it seems confusing and unnecessary to vary
this with the segment size.  On the other hand, the problem is that if
the segment size is larger than the default max_wal_size, we have to do
something different anyway.  Also, the reason for wanting a larger
segment size is that you expect a lot of WAL, so setting max_wal_size
larger by default caters to that.

Right now, we have max_wal_size = 1GB by default.  What should the
behavior be for wal_segment_size = 1GB?

> 2) Currently wal_segment_size is measured in GUC_UNIT_XBLOCKS, which
>    requires us to keep two copies of the underlying variable, one in
>    XBLOCKS one in bytes. I'd rather just have the byte variant.

It seems to me that one could structure the code to make do with the
existing variable.  I had a brief look at the patch, and I think some
more imaginative refactoring is possible.  If we want/need to change it,
I'd prefer using the _KB or _MB variants, so as to not to restrict
future size limits too much.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Hi,


To make it clear: I don't have a strong opinion on these, I'm happy
enough to commit the patch as is, minus one unrelated change. I just
think it should be discussed.


On 2017-08-30 07:01:54 -0400, Peter Eisentraut wrote:
> On 8/30/17 00:45, Andres Freund wrote:
> > 1) Currently the default for {min,max}_wal_size depends on the segment
> >    size. Given that the segment size is about to be configurable, that
> >    seems confusing.
> 
> On the one hand, I agree that it seems confusing and unnecessary to vary
> this with the segment size.  On the other hand, the problem is that if
> the segment size is larger than the default max_wal_size, we have to do
> something different anyway.

Well, rounding up to two segments, is different than rounding up to
64GB... I think there's a fair argument to be made that that's confusing
too.


> Also, the reason for wanting a larger segment size is that you expect
> a lot of WAL, so setting max_wal_size larger by default caters to
> that.

This I find unconvincing. If we want more autotuning, we should do that,
rather than define random things as that.


> Right now, we have max_wal_size = 1GB by default.  What should the
> behavior be for wal_segment_size = 1GB?

2GB.



> > 2) Currently wal_segment_size is measured in GUC_UNIT_XBLOCKS, which
> >    requires us to keep two copies of the underlying variable, one in
> >    XBLOCKS one in bytes. I'd rather just have the byte variant.
> 
> It seems to me that one could structure the code to make do with the
> existing variable. I had a brief look at the patch, and I think some
> more imaginative refactoring is possible.

That'd add yet more instructions to tight spinlock'ed paths, so I'm
loathe to do so. Or maybe I'm just missing what you're thinking about?


> If we want/need to change it, I'd prefer using the _KB or _MB
> variants, so as to not to restrict future size limits too much.

I'm a bit surprised at this one - if we want to add larger segment size
limits (which I don't think are useful), wouldn't it be just as
appropriate to extend the GUC infrastructure?


Greetings,

Andres Freund

On 08/30/2017 03:16 AM, Andres Freund wrote:
> On 2017-08-30 10:14:22 +0900, Michael Paquier wrote:
>> On Wed, Aug 30, 2017 at 10:06 AM, Andres Freund <andres@anarazel.de> wrote:
>>> On 2017-08-30 09:49:14 +0900, Michael Paquier wrote:
>>>> Do you think that we should worry about wal segment sizes higher than
>>>> 2GB? Support for int64 GUCs is not here yet.
>>>
>>> 1GB will be the limit anyway.
>>
>> Yeah, but imagine that we'd want to raise that even more up.
> 
> I'm doubtfull of that. But even if, it'd not be hard to GUC support.
> 

It's not hard - it's just a lot of copy-pasting of infrastructure code.
Incidentally, I already have a patch doing that, as we had to add that
support to XL, and I can submit it to PostgreSQL. Might save some boring
coding.

regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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