Re: basebackup.c's sendFile() ignores read errors

2019-09-06 Thread Robert Haas
On Fri, Sep 6, 2019 at 2:08 AM Jeevan Chalke wrote: > Attached patch for v10 and pre. The same v10 patch applies cleanly. > > Changes related to the page checksum verification is not present on v10 and > pre, and thus those changes are not applicable, so removed those. Also, > wal_segment_size is

Re: basebackup.c's sendFile() ignores read errors

2019-09-05 Thread Jeevan Chalke
On Thu, Sep 5, 2019 at 11:40 PM Robert Haas wrote: > On Fri, Aug 30, 2019 at 7:05 AM Jeevan Ladhe > wrote: > >> Fixed both comments in the attached patch. > > > > Thanks, the patch looks good to me. > > Here's a version of the patch with a further change to the wording of > the comment. I hope

Re: basebackup.c's sendFile() ignores read errors

2019-09-05 Thread Robert Haas
On Fri, Aug 30, 2019 at 7:05 AM Jeevan Ladhe wrote: >> Fixed both comments in the attached patch. > > Thanks, the patch looks good to me. Here's a version of the patch with a further change to the wording of the comment. I hope this is clearer. I think this needs to be back-patched all the way

Re: basebackup.c's sendFile() ignores read errors

2019-08-30 Thread Jeevan Ladhe
> > Fixed both comments in the attached patch. > Thanks, the patch looks good to me. Regards, Jeevan Ladhe

Re: basebackup.c's sendFile() ignores read errors

2019-08-30 Thread Jeevan Chalke
On Thu, Aug 29, 2019 at 3:17 PM Jeevan Ladhe wrote: > Hi Jeevan > > I had a look at the patch and this seems correct to me. > Thanks, Jeevan Ladhe. > > Few minor comments: > > + /* Check fread() error. */ > + CHECK_FREAD_ERROR(fp, pathbuf); > + > > The comments above the macro call at both the

Re: basebackup.c's sendFile() ignores read errors

2019-08-29 Thread Jeevan Ladhe
Hi Jeevan, On Wed, Aug 28, 2019 at 10:26 PM Jeevan Chalke < jeevan.cha...@enterprisedb.com> wrote: > > > On Tue, Aug 27, 2019 at 10:33 PM Robert Haas > wrote: > >> While reviewing a proposed patch to basebackup.c this morning, I found >> myself a bit underwhelmed by the quality of the code and c

Re: basebackup.c's sendFile() ignores read errors

2019-08-28 Thread Jeevan Chalke
On Tue, Aug 27, 2019 at 10:33 PM Robert Haas wrote: > While reviewing a proposed patch to basebackup.c this morning, I found > myself a bit underwhelmed by the quality of the code and comments in > basebackup.c's sendFile(). I believe it's already been pointed out > that the the retry logic here

basebackup.c's sendFile() ignores read errors

2019-08-27 Thread Robert Haas
While reviewing a proposed patch to basebackup.c this morning, I found myself a bit underwhelmed by the quality of the code and comments in basebackup.c's sendFile(). I believe it's already been pointed out that the the retry logic here is not particularly correct, and the comments demonstrate a pr