Re: Add SKIP LOCKED to VACUUM and ANALYZE

On Tue, Oct 02, 2018 at 02:22:42AM +0000, Bossart, Nathan wrote:
> On 10/1/18, 7:07 PM, "Michael Paquier" <michael(at)paquier(dot)xyz> wrote:
>> On Mon, Oct 01, 2018 at 03:37:01PM +0000, Bossart, Nathan wrote:
>>> Without the find_all_inheritors() stuff, I think we would just need to
>>> modify the ANALYZE documentation patch to say something like
>>>
>>> Specifies that ANALYZE should not wait for any conflicting locks
>>> to be released: if a relation cannot be locked immediately without
>>> waiting, the relation is skipped. Note that even with this
>>> option, ANALYZE can still block when opening the relation's
>>> indexes or when acquiring sample rows to prepare the statistics.
>>> Also, while ANALYZE ordinarily processes all leaf partitions of
>>> partitioned tables, this option will cause ANALYZE to skip all
>>> leaf partitions if there is a conflicting lock on the partitioned
>>> table.
>>
>> Yes, I think that we could live with something like that. I could give
>> this whole thing a shot, but this will have to wait until the commit
>> fest is closed as there are still many patches to classify. If you can
>> send a patch that's of course always helpful..
>
> Sure, here it is.

I was just playing with autovacuum and some inheritance trees, and
actually locking a child would cause an autovacuum worker to block if it
is analyzing the parent when acquiring row samples even now. Anyway,
coming back to the patch...

My comments are mostly about the documentation bits added:
- Inheritance trees have the same problem, so it would be nice to
mention them as well.
- One workaround, which we could document (?), would be to list leaves
of a partition tree individually so as their locks are checked one by
one.

+ Specifies that <command>ANALYZE</command> should not wait for any
+ conflicting locks to be released: if a relation cannot be locked

Perhaps it would be more precise to tell when "beginning to work on a
relation" because the restrictions in row samplings?

The rest of the patch looks clean to me.
--
Michael