Re: pg_basebackup and wal streaming

Better late than never (or?), here's the final cleanup of
pg_streamrecv for moving into the main distribution, per discussion
back in late dec or early jan. It also includes the "stream logs in
parallel to backup" part that was not completed on pg_basebackup.

Other than that, the only changes to pg_basebackup are the moving of a
couple of functions into streamutil.c to make them usable from both,
and the progress format output fix Fujii-san mentioned recently.

Should be complete except for Win32 support (needs thread/fork thing
for the background streaming feature. Shouldn't be too hard, and I
guess that falls on me anyway..) and the reference documentation.

And with no feedback to my question here
(http://archives.postgresql.org/pgsql-hackers/2011-02/msg00805.php), I
went with the "duplicate the macros that are needed to avoid loading
postgres.h" path.

Yes, I realize that this is far too late in the CF process really, but
I wanted to post it anyway... If it's too late to be acceptable it
should be possible to maintain this outside the main repository until
9.2, since it only changes frontend binaries. So I'm not actually
going to put it on the CF page unless someone else says that's a good
idea, to at least share the blame from Robert ;)

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Magnus Hagander wrote:
> Better late than never (or?), here's the final cleanup of
> pg_streamrecv for moving into the main distribution, per discussion
> back in late dec or early jan. It also includes the "stream logs in
> parallel to backup" part that was not completed on pg_basebackup.
>
> Other than that, the only changes to pg_basebackup are the moving of a
> couple of functions into streamutil.c to make them usable from both,
> and the progress format output fix Fujii-san mentioned recently.
>
> Should be complete except for Win32 support (needs thread/fork thing
> for the background streaming feature. Shouldn't be too hard, and I
> guess that falls on me anyway..) and the reference documentation.
>
> And with no feedback to my question here
> (http://archives.postgresql.org/pgsql-hackers/2011-02/msg00805.php), I
> went with the "duplicate the macros that are needed to avoid loading
> postgres.h" path.
>
> Yes, I realize that this is far too late in the CF process really, but
> I wanted to post it anyway... If it's too late to be acceptable it
> should be possible to maintain this outside the main repository until
> 9.2, since it only changes frontend binaries. So I'm not actually
> going to put it on the CF page unless someone else says that's a good
> idea, to at least share the blame from Robert ;)

Well, if you are going to stand behind it, the CF is not a requirement
and you can apply it.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On Fri, Feb 18, 2011 at 20:33, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> Magnus Hagander wrote:
>> Better late than never (or?), here's the final cleanup of
>> pg_streamrecv for moving into the main distribution, per discussion
>> back in late dec or early jan. It also includes the "stream logs in
>> parallel to backup" part that was not completed on pg_basebackup.
>>
>> Other than that, the only changes to pg_basebackup are the moving of a
>> couple of functions into streamutil.c to make them usable from both,
>> and the progress format output fix Fujii-san mentioned recently.
>>
>> Should be complete except for Win32 support (needs thread/fork thing
>> for the  background streaming feature. Shouldn't be too hard, and I
>> guess that falls on me anyway..) and the reference documentation.
>>
>> And with no feedback to my question here
>> (http://archives.postgresql.org/pgsql-hackers/2011-02/msg00805.php), I
>> went with the "duplicate the macros that are needed to avoid loading
>> postgres.h" path.
>>
>> Yes, I realize that this is far too late in the CF process really, but
>> I wanted to post it anyway... If it's too late to be acceptable it
>> should be possible to maintain this outside the main repository until
>> 9.2, since it only changes frontend binaries. So I'm not actually
>> going to put it on the CF page unless someone else says that's a good
>> idea, to at least share the blame from Robert ;)
>
> Well, if you are going to stand behind it, the CF is not a requirement
> and you can apply it.

I am, but I'm posting it here because I'd appreciate some review
before it goes in, to protect our users from my bugs :-)

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Hi,

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> Better late than never (or?), here's the final cleanup of
> pg_streamrecv for moving into the main distribution, per discussion
> back in late dec or early jan. It also includes the "stream logs in
> parallel to backup" part that was not completed on pg_basebackup.

And that's something I've been so interested in! It's only fair game
that I spend time reviewing after my insisting for having it :)

The programs (pg_basebackup and pg_receivexlog) both work as expected,
and show in the pg_stat_replication system view.

pg_basebackup -x option should be revised so that it's easier to make
the difference between streaming WAL while the base backup is ongoing
and fetching them at the end, taking the risk to corrupt the whole
backup as soon as wal_keep_segments is undersized.

-x, --xlog[=stream] include required WAL files in backup

It could be --xlog=stream|fetch or something that reads better.

The sent patch includes a binary called pg_receivexlog\(X\), but Magnus
told me on IRC that this is fixed already in his branch (a missing $ at
several places in the Makefile).

Now, on to the code itself.

I wonder if the segment_callback() routine would better be a while{}
loop rather than a recursive construct. Also, it looks like a lib
function but it's doing exit(1)…

Unfortunately I can't comment (or won't risk learning enough details
tonight to try to be smart here) on FindStreamingStart() implementation,
that seems crucial.

> Other than that, the only changes to pg_basebackup are the moving of a
> couple of functions into streamutil.c to make them usable from both,
> and the progress format output fix Fujii-san mentioned recently.

Check.

> Should be complete except for Win32 support (needs thread/fork thing
> for the background streaming feature. Shouldn't be too hard, and I
> guess that falls on me anyway..) and the reference documentation.

Yeah, StartLogStreamer() is still using fork() at the moment, I guess
you will have to change that prior to commit. Maybe you can reuse the
code found in src/bin/pg_dump/pg_backup_archiver.c, spawn_restore.

I can't wait to see the new streaming replication setup docs using that
integrated tool. Even if baring another step here, we still need to
rely on wal_keep_segments (for switching from the base backup to the
live standby), the critical window is so much reduced… and it's now
possible to prepare the standby using a single integrated command line.

Will the server refrain from recycling a WAL file when all receivers
sent_location are not known to be past the positions contained in it?
If that's the case, the documentation should talk about pg_receivexlog
as an alternative to archiving, relying on libpq. It that's not the
case, is there a good reason for that not being the case? (even if
that's not on this patch to fix that).

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

On 18.02.2011 12:02, Magnus Hagander wrote:
> Better late than never (or?), here's the final cleanup of
> pg_streamrecv for moving into the main distribution, per discussion
> back in late dec or early jan. It also includes the "stream logs in
> parallel to backup" part that was not completed on pg_basebackup.

Looks reasonable at a quick glance.

> + /* Already know when to stop, compare to the position we got */

That sentence sounds broken.

> + * The background stream will use it's own database connection so we can

s/it's/its/

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On 2011-02-20 21:37, Dimitri Fontaine wrote:
> Hi,
>
> Magnus Hagander<magnus(at)hagander(dot)net> writes:
>> Better late than never (or?), here's the final cleanup of
>> pg_streamrecv for moving into the main distribution, per discussion
>> back in late dec or early jan. It also includes the "stream logs in
>> parallel to backup" part that was not completed on pg_basebackup.
>
> I can't wait to see the new streaming replication setup docs using that
> integrated tool.
I just did some initial playing around with this tool to start testing
the latest syncrep patch. I'm time boxed for today, but just wanted to
say: great tool.

mgrid(at)standby1:~/off/postgresql$ pg_basebackup -x -D /data -vP -h
192.168.73.34
xlog start point: 0/7000020
34537/18152 kb g(100%) 1/1 tablespaces
xlog end point: 0/7014740
pg_basebackup: base backup completed.
mgrid(at)standby1:~/off/postgresql$ pg_ctl -D /data -l logfile start
server starting
mgrid(at)standby1:~/off/postgresql$ psql postgres
psql (9.1devel)
Type "help" for help.

postgres=# \d
List of relations
Schema | Name | Type | Owner
--------+------+-------+-------
public | aap | table | mgrid
(1 row)

regards,
Yeb Havinga

On Sun, Feb 20, 2011 at 21:37, Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr> wrote:
> Hi,
>
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> Better late than never (or?), here's the final cleanup of
>> pg_streamrecv for moving into the main distribution, per discussion
>> back in late dec or early jan. It also includes the "stream logs in
>> parallel to backup" part that was not completed on pg_basebackup.
>
> And that's something I've been so interested in!  It's only fair game
> that I spend time reviewing after my insisting for having it :)
>
> The programs (pg_basebackup and pg_receivexlog) both work as expected,
> and show in the pg_stat_replication system view.
>
> pg_basebackup -x option should be revised so that it's easier to make
> the difference between streaming WAL while the base backup is ongoing
> and fetching them at the end, taking the risk to corrupt the whole
> backup as soon as wal_keep_segments is undersized.
>
>  -x, --xlog[=stream]       include required WAL files in backup
>
> It could be --xlog=stream|fetch or something that reads better.

Yeha, that's probably true. I wanted to avoid making it mandatory, but
it's actually easier this way. Will change it to that.

> Now, on to the code itself.
>
> I wonder if the segment_callback() routine would better be a while{}
> loop rather than a recursive construct.  Also, it looks like a lib
> function but it's doing exit(1)…

Actually, it's even better to just reorder the checks in the other
order - that way we don't need a loop *or* a self-call.

> Unfortunately I can't comment (or won't risk learning enough details
> tonight to try to be smart here) on FindStreamingStart() implementation,
> that seems crucial.

It is - so if you can find the time to, that would be great...

> Will the server refrain from recycling a WAL file when all receivers
> sent_location are not known to be past the positions contained in it?
> If that's the case, the documentation should talk about pg_receivexlog
> as an alternative to archiving, relying on libpq.  It that's not the
> case, is there a good reason for that not being the case? (even if
> that's not on this patch to fix that).

No, not at this point. It would be nice to have that option in the future...

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

On Sat, Feb 26, 2011 at 16:28, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> On Sun, Feb 20, 2011 at 21:37, Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr> wrote:
>> Hi,
>>
>> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>>> Better late than never (or?), here's the final cleanup of
>>> pg_streamrecv for moving into the main distribution, per discussion
>>> back in late dec or early jan. It also includes the "stream logs in
>>> parallel to backup" part that was not completed on pg_basebackup.
>>
>> And that's something I've been so interested in!  It's only fair game
>> that I spend time reviewing after my insisting for having it :)
>>
>> The programs (pg_basebackup and pg_receivexlog) both work as expected,
>> and show in the pg_stat_replication system view.
>>
>> pg_basebackup -x option should be revised so that it's easier to make
>> the difference between streaming WAL while the base backup is ongoing
>> and fetching them at the end, taking the risk to corrupt the whole
>> backup as soon as wal_keep_segments is undersized.
>>
>>  -x, --xlog[=stream]       include required WAL files in backup
>>
>> It could be --xlog=stream|fetch or something that reads better.
>
> Yeha, that's probably true. I wanted to avoid making it mandatory, but
> it's actually easier this way. Will change it to that.
>
>
>> Now, on to the code itself.
>>
>> I wonder if the segment_callback() routine would better be a while{}
>> loop rather than a recursive construct.  Also, it looks like a lib
>> function but it's doing exit(1)…
>
> Actually, it's even better to just reorder the checks in the other
> order - that way we don't need a loop *or* a self-call.
>
>
>> Unfortunately I can't comment (or won't risk learning enough details
>> tonight to try to be smart here) on FindStreamingStart() implementation,
>> that seems crucial.
>
> It is - so if you can find the time to, that would be great...

Attached is an updated version of the patch that includes these
changes, as well as Windows support and an initial cut at a ref page
for pg_receivexlog (needs some more detail still).

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

On 2011-02-26 18:19, Magnus Hagander wrote:
> Attached is an updated version of the patch that includes these
> changes, as well as Windows support and an initial cut at a ref page
> for pg_receivexlog (needs some more detail still).
I'm testing a bit more (with the previous version, sorry) and got the
following while doing a stream backup from a cluster that was at that
moment doing a pgbench run with 1 synchronous standby.

mgrid(at)mg79:~$ pg_basebackup --xlog=stream -D /data -vP -h mg73 -U repuser
Password:
xlog start point: 15/720000C8
pg_basebackup: starting background WAL receiver
pg_basebackup: got WAL data offset 14744, expected 16791960424 )
5148915/5148026 kb g(100%) 1/1 tablespaces
xlog end point: 15/80568878
pg_basebackup: waiting for background process to finish streaming...
pg_basebackup: child process exited with error 1

The fetch variant worked ok.

mgrid(at)mg79:~$ pg_basebackup --xlog -D /data -vP -h mg73 -U repuser
Password:
xlog start point: 15/A2000020
5482029/5153458 kb g(100%) 1/1 tablespaces
xlog end point: 15/B51D0230
pg_basebackup: base backup completed.

I'm in total monkey test mode here, so I don't even know if I'm not
supposed to do the streaming variant while other stuff is going on.

regards,
Yeb Havinga

On Sat, Feb 26, 2011 at 20:48, Yeb Havinga <yebhavinga(at)gmail(dot)com> wrote:
> On 2011-02-26 18:19, Magnus Hagander wrote:
>>
>> Attached is an updated version of the patch that includes these
>> changes, as well as Windows support and an initial cut at a ref page
>> for pg_receivexlog (needs some more detail still).
>
> I'm testing a bit more (with the previous version, sorry) and got the
> following while doing a stream backup from a cluster that was at that moment
> doing a pgbench run with 1 synchronous standby.
>
> mgrid(at)mg79:~$ pg_basebackup --xlog=stream -D /data -vP -h mg73 -U repuser
> Password:
> xlog start point: 15/720000C8
> pg_basebackup: starting background WAL receiver
> pg_basebackup: got WAL data offset 14744, expected 16791960424        )
> 5148915/5148026 kb g(100%) 1/1 tablespaces
> xlog end point: 15/80568878
> pg_basebackup: waiting for background process to finish streaming...
> pg_basebackup: child process exited with error 1

Hmm, strange. What platform are you on?

I saw something similar *once* on Windows, but it then passed my tests
a lot of times in a row so I figured it was just a "didn't clean
properly" thing. Clearly there's a bug around.

What's the size of the latest WAL file that it did work on? Is it
16791960424 bytes? That's way way to large, but perhaps it's not
switching segment properly? (that value is supposedly the current
write position in the file..)

> I'm in total monkey test mode here, so I don't even know if I'm not supposed
> to do the streaming variant while other stuff is going on.

Oh yes, that's one of the main reasons to use it, so you should
definitely be able to do that!

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

On 2011-02-26 20:59, Magnus Hagander wrote:
> On Sat, Feb 26, 2011 at 20:48, Yeb Havinga<yebhavinga(at)gmail(dot)com> wrote:
>> On 2011-02-26 18:19, Magnus Hagander wrote:
>>> Attached is an updated version of the patch that includes these
>>> changes, as well as Windows support and an initial cut at a ref page
>>> for pg_receivexlog (needs some more detail still).
>> I'm testing a bit more (with the previous version, sorry) and got the
>> following while doing a stream backup from a cluster that was at that moment
>> doing a pgbench run with 1 synchronous standby.
>>
>> mgrid(at)mg79:~$ pg_basebackup --xlog=stream -D /data -vP -h mg73 -U repuser
>> Password:
>> xlog start point: 15/720000C8
>> pg_basebackup: starting background WAL receiver
>> pg_basebackup: got WAL data offset 14744, expected 16791960424 )
>> 5148915/5148026 kb g(100%) 1/1 tablespaces
>> xlog end point: 15/80568878
>> pg_basebackup: waiting for background process to finish streaming...
>> pg_basebackup: child process exited with error 1
> Hmm, strange. What platform are you on?
Both master and backup are on
Linux mg79 2.6.31-22-server #60-Ubuntu SMP Thu May 27 03:42:09 UTC 2010
x86_64 GNU/Linux

If I run a streaming backup with idle master server, things are ok.

I can repeat the error by doing a pgbench run on the master (with a
syncrep standby, don't know if that's of importance, other that there is
another walsender) and then running a streaming pg_basebackup.

> I saw something similar *once* on Windows, but it then passed my tests
> a lot of times in a row so I figured it was just a "didn't clean
> properly" thing. Clearly there's a bug around.
>
> What's the size of the latest WAL file that it did work on? Is it
> 16791960424 bytes? That's way way to large, but perhaps it's not
> switching segment properly? (that value is supposedly the current
> write position in the file..)
The files from that specific run are gone.

mgrid(at)mg79:~$ pg_basebackup --xlog=stream -D /data -vP -h mg73 -U repuser
Password:
xlog start point: 47/8E81F300
pg_basebackup: starting background WAL receiver
pg_basebackup: got WAL data offset 41432, expected 168186486424 )
4763109/4762428 kb g(100%) 1/1 tablespaces
xlog end point: 47/9D17FFE0
pg_basebackup: waiting for background process to finish streaming...
pg_basebackup: child process exited with error 1

About the file sizes, every WAL file on the master is 16M big, though
the sum of the size of all WAL files on the master is close to the 16G
number. Maybe a coincidence..
/dev/sdc1 20970612 16798508 4172104 81% /xlog

New initdb, pgbench with smaller db, fresh /xlog:

mgrid(at)mg79:~$ pg_basebackup --xlog=stream -D /data -vP -h mg73 -U repuser
Password:
xlog start point: 0/8C6474E8
pg_basebackup: starting background WAL receiver
pg_basebackup: got WAL data offset 10488, expected 167877046397 )
1564067/1563386 kb g(100%) 1/1 tablespaces
xlog end point: 0/99007798
pg_basebackup: waiting for background process to finish streaming...
pg_basebackup: child process exited with error 1

yes a coincidence..

/dev/sdc1 20G 2.6G 18G 13% /xlog

Another test with no sync standby server, only the master with a pgbench
running:

mgrid(at)mg79:~$ pg_basebackup --xlog=stream -D /data -vP -h mg73 -U repuser
Password:
xlog start point: 0/D5002120
pg_basebackup: starting background WAL receiver
pg_basebackup: got WAL data offset 23752, expected 168009686397 )
1572173/1570348 kb g(100%) 1/1 tablespaces
xlog end point: 0/EC456968
pg_basebackup: waiting for background process to finish streaming...
pg_basebackup: child process exited with error 1

Stopping pgbench and starting a basebackup., then it finishes correctly
after a while (with in between something that looks like ~ 2 minutes
inactivity).

regards,
Yeb Havinga

On 2011-02-18 11:02, Magnus Hagander wrote:
> Better late than never (or?), here's the final cleanup of
> pg_streamrecv for moving into the main distribution, per discussion
> back in late dec or early jan. It also includes the "stream logs in
> parallel to backup" part that was not completed on pg_basebackup.
Is it a welcome idea to add a -X argument to specify a seperate xlog
directory like initdb -X ?

regards,
Yeb Havinga

On Fri, Mar 4, 2011 at 15:23, Yeb Havinga <yebhavinga(at)gmail(dot)com> wrote:
> On 2011-02-18 11:02, Magnus Hagander wrote:
>>
>> Better late than never (or?), here's the final cleanup of
>> pg_streamrecv for moving into the main distribution, per discussion
>> back in late dec or early jan. It also includes the "stream logs in
>> parallel to backup" part that was not completed on pg_basebackup.
>
> Is it a welcome idea to add a -X argument to specify a seperate xlog
> directory like initdb -X ?

Probably not a bad idea - for a future enhancement ;)

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Magnus Hagander wrote:
> On Fri, Mar 4, 2011 at 15:23, Yeb Havinga <yebhavinga(at)gmail(dot)com> wrote:
> > On 2011-02-18 11:02, Magnus Hagander wrote:
> >>
> >> Better late than never (or?), here's the final cleanup of
> >> pg_streamrecv for moving into the main distribution, per discussion
> >> back in late dec or early jan. It also includes the "stream logs in
> >> parallel to backup" part that was not completed on pg_basebackup.
> >
> > Is it a welcome idea to add a -X argument to specify a seperate xlog
> > directory like initdb -X ?
>
> Probably not a bad idea - for a future enhancement ;)

A TODO? Wording?

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On Fri, Mar 11, 2011 at 16:19, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> Magnus Hagander wrote:
>> On Fri, Mar 4, 2011 at 15:23, Yeb Havinga <yebhavinga(at)gmail(dot)com> wrote:
>> > On 2011-02-18 11:02, Magnus Hagander wrote:
>> >>
>> >> Better late than never (or?), here's the final cleanup of
>> >> pg_streamrecv for moving into the main distribution, per discussion
>> >> back in late dec or early jan. It also includes the "stream logs in
>> >> parallel to backup" part that was not completed on pg_basebackup.
>> >
>> > Is it a welcome idea to add a -X argument to specify a seperate xlog
>> > directory like initdb -X ?
>>
>> Probably not a bad idea - for a future enhancement ;)
>
> A TODO?  Wording?

"Add -X parameter to pg_basebackup to specify a different directory
for px_xlog, like initdb"

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

On Sat, Feb 26, 2011 at 20:59, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> On Sat, Feb 26, 2011 at 20:48, Yeb Havinga <yebhavinga(at)gmail(dot)com> wrote:
>> On 2011-02-26 18:19, Magnus Hagander wrote:
>>>
>>> Attached is an updated version of the patch that includes these
>>> changes, as well as Windows support and an initial cut at a ref page
>>> for pg_receivexlog (needs some more detail still).
>>
>> I'm testing a bit more (with the previous version, sorry) and got the
>> following while doing a stream backup from a cluster that was at that moment
>> doing a pgbench run with 1 synchronous standby.
>>
>> mgrid(at)mg79:~$ pg_basebackup --xlog=stream -D /data -vP -h mg73 -U repuser
>> Password:
>> xlog start point: 15/720000C8
>> pg_basebackup: starting background WAL receiver
>> pg_basebackup: got WAL data offset 14744, expected 16791960424        )
>> 5148915/5148026 kb g(100%) 1/1 tablespaces
>> xlog end point: 15/80568878
>> pg_basebackup: waiting for background process to finish streaming...
>> pg_basebackup: child process exited with error 1
>
> Hmm, strange. What platform are you on?
>
> I saw something similar *once* on Windows, but it then passed my tests
> a lot of times in a row so I figured it was just a "didn't clean
> properly" thing. Clearly there's a bug around.
>
> What's the size of the latest WAL file that it did work on? Is it
> 16791960424 bytes? That's way way to large, but perhaps it's not
> switching segment properly? (that value is supposedly the current
> write position in the file..)
>
>
>> I'm in total monkey test mode here, so I don't even know if I'm not supposed
>> to do the streaming variant while other stuff is going on.
>
> Oh yes, that's one of the main reasons to use it, so you should
> definitely be able to do that!
>

I've posted a new version of this patch at
http://archives.postgresql.org/pgsql-hackers/2011-08/msg00776.php -
forgot there was an open thread on this, sorry.

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/