From: | Magnus Hagander <magnus(at)hagander(dot)net> |
---|---|
To: | Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> |
Cc: | pgsql-committers <pgsql-committers(at)postgresql(dot)org> |
Subject: | Re: pgsql: Fix base backup rate limiting in presence of slow i/o |
Date: | 2017-01-01 12:23:40 |
Message-ID: | CABUevEwFzOp4DPQL9K68dpKOEW6b2cYz=WKJ71Vkscs0=Gbp2A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers |
On Thu, Dec 22, 2016 at 11:10 AM, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
wrote:
> On 21 December 2016 at 20:20, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> > On Wed, Dec 21, 2016 at 6:55 PM, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
> >> basebackup.c: In function ‘throttle’:
> >> basebackup.c:1284:8: warning: variable ‘wait_result’ set but not used
> >> [-Wunused-but-set-variable]
> >> int wait_result;
> >
> > Interesting.
> >
> > ff44fba4 replaced the latch in walsender, which was not backported (of
> > course).
> >
> > But it also added a CHECK_FOR_INTERRUPTS there.
> >
> > 9.4 does not have a CHECK_FOR_INTERRUPTS anywhere near there. Perhaps
> what's
> > really needed is to put one of those in regardless?
> >
>
> Yeah, it seems reasonable that there should be a CHECK_FOR_INTERRUPTS()
> there.
>
> I'm not really familiar with that code, but it looks like the signal
> handler in 9.4 might not set that latch, so throttle() would have to
> rely on setting ImmediateInterruptOK to make the sleep interruptable,
> in which case the wait result would be irrelevant, right?
>
Sorry for the delay in getting back to this. Looking it over again, yes, I
think you are right, and will commit a patch that just removes the check
for result.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
From | Date | Subject | |
---|---|---|---|
Next Message | Magnus Hagander | 2017-01-01 12:25:23 | pgsql: Silence compiler warning |
Previous Message | Tom Lane | 2016-12-31 23:39:18 | pgsql: Fix unstable regression test results. |