Re: Proposed patch: Smooth replication during VACUUM FULL

On Sat, Apr 30, 2011 at 11:48 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Jaime Casanova <jaime(at)2ndquadrant(dot)com> writes:
>> On Sat, Apr 30, 2011 at 1:19 PM, Gabriele Bartolini
>>> I have noticed that during VACUUM FULL on reasonably big tables, replication
>>> lag climbs. In order to smooth down the replication lag, I propose the
>>> attached patch which enables vacuum delay for VACUUM FULL.
>
>> AFAICS, the problem is that those operations involve the rebuild of
>> tables, so we can't simply stop in the middle and wait because we will
>> need to hold a strong lock more time... also the patch seems to be
>> only doing something for CLUSTER and not for VACUUM FULL.
>> or am i missing something?
>
> No, actually it would have no effect on CLUSTER because VacuumCostActive
> wouldn't be set.

Ah, good point, so the patch is accurate even though very short.

> I think this is basically fixing an oversight in the
> patch that changed VACUUM FULL into a variant of CLUSTER.  We used to
> use vacuum_delay_point() in the main loops in old-style VACUUM FULL,
> but forgot to consider doing so in the CLUSTER-ish implementation.
> The argument about holding locks longer doesn't seem relevant to me:
> enabling delays during VACUUM FULL would've had that effect in the old
> implementation, too, but nobody ever complained about that, and besides
> the feature isn't enabled by default.
>
> A bigger objection to this patch is that it seems quite incomplete.
> I'm not sure there's much point in adding delays to the first loop of
> copy_heap_data() without also providing for delays inside the sorting
> code and the eventual index rebuilds; which will make the patch
> significantly more complicated and invasive.

The patch puts the old behaviour of vacuum delay back into VACUUM FULL
and seems worth backpatching to 9.0 and 9.1 to me, since it is so
simple.

Previously there wasn't any delay in the sort or indexing either, so
it's a big ask to put that in now and it would also make backpatching
harder.

I think we should backpatch this now, but work on an additional delay
in sort and index for 9.2 to "complete" this thought. ISTM a good idea
for us to add similar delay code to all "maintenance" commands, should
the user wish to use it (9.2+).

> Another question is whether this is the right place to be looking
> at all.  If Gabriele's setup can't keep up with replication when a
> VAC FULL is running, then it can't keep up when under load, period.
> This seems like a pretty band-aid-ish response to that sort of problem.

This isn't about whether the system can cope with the load, its about
whether replication lag is affected by the load.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services