Re: streaming header too small

On Mon, Jan 9, 2012 at 12:00, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> On Mon, Jan 9, 2012 at 11:09, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
>> On Mon, Jan 9, 2012 at 07:34, Jaime Casanova <jaime(at)2ndquadrant(dot)com> wrote:
>>> Hi,
>>>
>>> I was trying pg_basebackup on head, i used this command:
>>> """
>>> postgres(at)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?

Patch applied.

Realized there is no need to backpatch, because this code didn't even
exist in 9.1. The streaming mode of pg_basebackup (which is the only
affected one) didn't exist then...

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