Re: [HACKERS] Reversed sync check in pg_receivewal

2017-04-12 Thread Magnus Hagander
On Tue, Apr 11, 2017 at 6:51 PM, Tom Lane wrote: > Magnus Hagander writes: > > Something like the attached? > > Not sure about > > + * All methods that have a failure path will set errno on failure. > > Given that you've got a getlasterror method, I

Re: [HACKERS] Reversed sync check in pg_receivewal

2017-04-11 Thread Tom Lane
Magnus Hagander writes: > Something like the attached? Not sure about + * All methods that have a failure path will set errno on failure. Given that you've got a getlasterror method, I don't think that's really the API invariant is it? If it were, you'd just have the

Re: [HACKERS] Reversed sync check in pg_receivewal

2017-04-11 Thread Magnus Hagander
On Tue, Apr 11, 2017 at 3:53 PM, Tom Lane wrote: > Magnus Hagander writes: > > On Tue, Apr 11, 2017 at 3:19 PM, Tom Lane wrote: > >> I think the patch is correct, but if there's any documentation of the > >> walmethod APIs that would

Re: [HACKERS] Reversed sync check in pg_receivewal

2017-04-11 Thread Tom Lane
Magnus Hagander writes: > On Tue, Apr 11, 2017 at 3:19 PM, Tom Lane wrote: >> I think the patch is correct, but if there's any documentation of the >> walmethod APIs that would allow one to assert which side of the API got >> this wrong, I sure don't see

Re: [HACKERS] Reversed sync check in pg_receivewal

2017-04-11 Thread Magnus Hagander
On Tue, Apr 11, 2017 at 3:19 PM, Tom Lane wrote: > Magnus Hagander writes: > > Attached patch reverses the check, and adds a failure message. I'd > > appreciate a quick review in case I have the logic backwards in my > head... > > I think the patch is

Re: [HACKERS] Reversed sync check in pg_receivewal

2017-04-11 Thread Aleksander Alekseev
Hi Magnus, > Attached patch reverses the check, and adds a failure message. I'd > appreciate a quick review in case I have the logic backwards in my head... Well, I can state that `make check-world` passes on my laptop and that code seems to be right. However documentation to WalWriteMethod

Re: [HACKERS] Reversed sync check in pg_receivewal

2017-04-11 Thread Michael Paquier
On Tue, Apr 11, 2017 at 9:41 PM, Magnus Hagander wrote: > This bug seems to have snuck in there with the introduction of walmethods. > AFAICT we are testing the result of sync() backwards, so whenever a partial > segment exists for pg_receivewal, it will fail. It will then

Re: [HACKERS] Reversed sync check in pg_receivewal

2017-04-11 Thread Tom Lane
Magnus Hagander writes: > Attached patch reverses the check, and adds a failure message. I'd > appreciate a quick review in case I have the logic backwards in my head... I think the patch is correct, but if there's any documentation of the walmethod APIs that would allow one

[HACKERS] Reversed sync check in pg_receivewal

2017-04-11 Thread Magnus Hagander
This bug seems to have snuck in there with the introduction of walmethods. AFAICT we are testing the result of sync() backwards, so whenever a partial segment exists for pg_receivewal, it will fail. It will then unlink the file, so when it retries 5 seconds later it works. It also doesn't log the