|From:||Michael Paquier <michael(at)paquier(dot)xyz>|
|To:||"Bossart, Nathan" <bossartn(at)amazon(dot)com>|
|Cc:||"pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, Lyes Ameddah <lyes(dot)amd(at)gmail(dot)com>|
|Subject:||Re: Add SKIP LOCKED to VACUUM and ANALYZE|
|Views:||Raw Message | Whole Thread | Download mbox|
On Wed, Sep 05, 2018 at 03:24:21PM +0000, Bossart, Nathan wrote:
> And here it is. Here is a summary of the notable changes:
> 1) Patches v8-0003 and v8-0008 have been discarded. These patches
> added SKIP_LOCKED behavior when opening a relation's indexes.
> Instead, I've documented that VACUUM and ANALYZE may still block
> on indexes in v9-0007.
> 2) Patches v8-0004 and v8-0005 have been discarded, as they have
> already been committed.
> 3) Patch v8-0011 has been discarded. As previously noted, VACUUM
> (SKIP_LOCKED, FULL) is already handled in vacuum_rel(), so no
> changed are required to cluster_rel(). However, we will need
> something similar to v8-0011 if we ever add SKIP_LOCKED to
> 4) The option has been renamed to SKIP_LOCKED (with the underscore)
> for consistency with the DISABLE_PAGE_SKIPPING option.
> 5) In the documentation, I've listed the caveats for SKIP_LOCKED and
> partitioned tables. I tried to make all the new documentation as
> concise as possible.
Thanks for the new patches. So I have begun looking at the full set,
beginning by 0001 which introduces a new common routine to get the
elevel associated to a skipped relation. I have been chewing on this
one, and I think that the patch could do more in terms of refactoring by
introducing one single routine able to open a relation which is going to
be vacuumed or analyzed. This removes quite a lot of duplication
between analyze_rel() and vacuum_rel() when it comes to using
try_relation_open(). The result is attached, and that makes the code
closer to what the recently-added vacuum_is_relation_owner does.
Nathan, what do you think?
Please note that I am on the edge of discarding the stuff in
find_all_inheritors and such, as well as not worrying about the case of
analyze which could block for some index columns, but I have not spent
much time studying this part yet. Still the potential issues around
complicating this code, particularly when things come to having a
partial analyze or vacuum done rather scares me.
|Next Message||Amit Kapila||2018-09-27 08:00:13||Re: Problem while setting the fpw with SIGHUP|
|Previous Message||Amit Langote||2018-09-27 07:14:35||Re: Postgres 11 release notes|