On Mon, Jan 9, 2012 at 11:09, Magnus Hagander <mag...@hagander.net> wrote:
> On Mon, Jan 9, 2012 at 07:34, Jaime Casanova <ja...@2ndquadrant.com> wrote:
>> Hi,
>>
>> I was trying pg_basebackup on head, i used this command:
>> """
>> postgres@jaime:/usr/local/pgsql/9.2$ bin/pg_basebackup -D $PWD/data2
>> -x stream -P -p 54392
>> """
>>
>> i got this error
>> """
>> 19093/19093 kB (100%), 1/1 tablespace
>> pg_basebackup: streaming header too small: 17
>> pg_basebackup: child process exited with error 1
>> """
>>
>> now, this streaming header size is defined in
>> src/bin/pg_basebackup/receivelog.c as "#define STREAMING_HEADER_SIZE
>> (1+8+8+8)", so WTF is this?
>> what are these numbers? shouldn't be at least a comment explaining
>> those? more important it's seems obvious something broke that, unless
>
> Those numbers are the size of WalDataMessageHeader - a struct which is
> not available in the frontend, or at least wasn't at the time.
>
>> i misunderstood something which is completely possible, and that the
>> way is do it it will broke again in the future if the header change
>
> Without looking at the details, I'm pretty sure it's the keepalive
> message patch (64233902d22ba42846397cb7551894217522fad4).That one does
> introduce a new message that's exactly that size.
>
> pg_basebackup assumes the only kind of messages that can arrive are
> the data messages, and this is no longer true. But if you check the
> code for pg_basebackup, you'll see it checks the size of the message
> *before* it checks the type of the message, which is why you get a
> misleading error.
>
> I'll dig into the details later - but you could try backing out that
> patch to confirm if that's the problem.

Confirmed that is it, and attached are two patches to fix it. The
first one I intend to backport to 9.1, since it just fixes the error
message. The other one is for 9.2. I'll also look at a better way to
get that structure size.  comments?

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/
From 99764c7132019c870e36ac56385500b8d5b454ed Mon Sep 17 00:00:00 2001
From: Magnus Hagander <mag...@hagander.net>
Date: Mon, 9 Jan 2012 11:53:38 +0100
Subject: [PATCH 1/2] Reorder check for streaming message and header size

This produces a more relevant error message when talking to a 9.2
server that sends keepalive messages.
---
 src/bin/pg_basebackup/receivelog.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c
index c18db4f..d76eed6 100644
--- a/src/bin/pg_basebackup/receivelog.c
+++ b/src/bin/pg_basebackup/receivelog.c
@@ -374,18 +374,18 @@ ReceiveXlogStream(PGconn *conn, XLogRecPtr startpos, uint32 timeline, char *sysi
 					progname, PQerrorMessage(conn));
 			return false;
 		}
-		if (r < STREAMING_HEADER_SIZE + 1)
-		{
-			fprintf(stderr, _("%s: streaming header too small: %i\n"),
-					progname, r);
-			return false;
-		}
 		if (copybuf[0] != 'w')
 		{
 			fprintf(stderr, _("%s: unrecognized streaming header: \"%c\"\n"),
 					progname, copybuf[0]);
 			return false;
 		}
+		if (r < STREAMING_HEADER_SIZE + 1)
+		{
+			fprintf(stderr, _("%s: streaming header too small: %i\n"),
+					progname, r);
+			return false;
+		}
 
 		/* Extract WAL location for this block */
 		memcpy(&blockpos, copybuf + 1, 8);
-- 
1.7.5.4

From 0c590630575014c1bc5a2ed700b802000d6a24ac Mon Sep 17 00:00:00 2001
From: Magnus Hagander <mag...@hagander.net>
Date: Mon, 9 Jan 2012 11:57:56 +0100
Subject: [PATCH 2/2] Accept (and ignore) keepalive messages in pg_basebackup

---
 src/bin/pg_basebackup/receivelog.c |   18 +++++++++++++++++-
 1 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c
index d76eed6..c5579ee 100644
--- a/src/bin/pg_basebackup/receivelog.c
+++ b/src/bin/pg_basebackup/receivelog.c
@@ -35,6 +35,7 @@
 
 /* Size of the streaming replication protocol header */
 #define STREAMING_HEADER_SIZE (1+8+8+8)
+#define STREAMING_KEEPALIVE_SIZE (1+8+8)
 
 const XLogRecPtr InvalidXLogRecPtr = {0, 0};
 
@@ -374,7 +375,22 @@ ReceiveXlogStream(PGconn *conn, XLogRecPtr startpos, uint32 timeline, char *sysi
 					progname, PQerrorMessage(conn));
 			return false;
 		}
-		if (copybuf[0] != 'w')
+		if (copybuf[0] == 'k')
+		{
+			/*
+			 * keepalive message, sent in 9.2 and newer. We just ignore
+			 * this message completely, but need to forward past it
+			 * in our reading.
+			 */
+			if (r != STREAMING_KEEPALIVE_SIZE)
+			{
+				fprintf(stderr, _("%s: keepalive message is incorrect size: %i\n"),
+						progname, r);
+				return false;
+			}
+			continue;
+		}
+		else if (copybuf[0] != 'w')
 		{
 			fprintf(stderr, _("%s: unrecognized streaming header: \"%c\"\n"),
 					progname, copybuf[0]);
-- 
1.7.5.4

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

Reply via email to