|From:||Beena Emerson <memissemerson(at)gmail(dot)com>|
|To:||Michael Paquier <michael(dot)paquier(at)gmail(dot)com>|
|Cc:||Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>|
|Subject:||Re: increasing the default WAL segment size|
|Views:||Raw Message | Whole Thread | Download mbox|
On Tue, Jan 17, 2017 at 12:18 PM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com
> On Sun, Jan 8, 2017 at 9:52 AM, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>
> >> I agree pretty_print_kb would have been a better for this function.
> >> However, I have realised that using the show hook and this function is
> >> not suitable and have found a better way of handling the removal of
> >> GUC_UNIT_XSEGS which no longer needs this function : using the
> >> GUC_UNIT_KB, convert the value in bytes to wal segment count instead in
> >> the assign hook. The next version of patch will use this.
> > ... it sounds like you're going back to exposing KB to users, and that's
> > that really matters.
> >> IMHO it'd be better to use the n & (n-1) check detailed at .
> That would be better.
> So I am looking at the proposed patch, though there have been reviews
> the patch was in "Needs Review" state, and as far as I can see it is a
> couple of things for frontends. Just by grepping for XLOG_SEG_SIZE I
> have spotted the following problems:
> - pg_standby uses it to know about the next segment available.
Yes. I am aware of this and had mentioned it in my post.
> - pg_receivexlog still uses it in segment handling.
> It may be a good idea to just remove XLOG_SEG_SIZE and fix the code
> paths that fail to compile without it, frontend utilities included
> because a lot of them now rely on the value coded in xlog_internal.h,
> but with this patch the value is set up in the context of initdb. And
> this would induce major breakages in many backup tools, pg_rman coming
> first in mind... We could replace it with for example a macro that
> frontends could use to check if the size of the WAL segment is in a
> valid range if the tool does not have direct access to the Postgres
> instance (aka the size of the WAL segment used there) as there are as
> well offline tools.
I will see whats the best way to do this.
> -#define XLogSegSize ((uint32) XLOG_SEG_SIZE)
> +extern uint32 XLogSegSize;
> +#define XLOG_SEG_SIZE XLogSegSize
> This bit is really bad for frontend declaring xlog_internal.h...
> --- a/src/bin/pg_test_fsync/pg_test_fsync.c
> +++ b/src/bin/pg_test_fsync/pg_test_fsync.c
> @@ -62,7 +62,7 @@ static const char *progname;
> static int secs_per_test = 5;
> static int needs_unlink = 0;
> -static char full_buf[XLOG_SEG_SIZE],
> +static char full_buf[DEFAULT_XLOG_SEG_SIZE],
> This would make sense as a new option of pg_test_fsync.
> A performance study would be a good idea as well. Regarding the
> generic SHOW command in the replication protocol, I may do it for next
> CF, I have use cases for it in my pocket.
Thank you for your review.
I have already made patch for the generic SHOW replication command
(attached) and am working on the new initdb patch based on that.
I have not yet fixed the pg_standby issue. I am trying to address all the
comments and bugs still.
Have a Great Day!
|Next Message||Michael Paquier||2017-01-17 07:08:22||Re: increasing the default WAL segment size|
|Previous Message||Michael Paquier||2017-01-17 06:50:29||Re: [PATCH] Refactor "if(strspn(str, ...) == strlen(str)" code|