Re: BUG #14941: Vacuum crashes

On 3/7/18, 9:34 AM, "Bossart, Nathan" <bossartn@amazon.com> wrote:
> On 3/6/18, 11:04 PM, "Michael Paquier" <michael@paquier.xyz> wrote:
>> +       if (!(options & VACOPT_SKIP_LOCKED))
>> +           relid = RangeVarGetRelid(vrel->relation, AccessShareLock,
>> false);
>> +       else
>> +       {
>> +           relid = RangeVarGetRelid(vrel->relation, NoLock, false);
>> Yeah, I agree with Andres that getting all this logic done in
>> RangeVarGetRelidExtended would be cleaner.  Let's see where the other
>> thread leads us to:
>> https://www.postgresql.org/message-id/20180306005349.b65whmvj7z6hbe2y%40alap3.anarazel.de
>
> I had missed that thread.  Thanks for pointing it out.

I've noticed one more problem with ACCESS EXCLUSIVE.  If an ACCESS
EXCLUSIVE lock is held on a child relation of a partitioned table,
an ANALYZE on the partitioned table will be blocked at
acquire_inherited_sample_rows().

static int
acquire_inherited_sample_rows(Relation onerel, int elevel,
                              HeapTuple *rows, int targrows,
                              double *totalrows, double *totaldeadrows)
{
...
    /*
     * Find all members of inheritance set.  We only need AccessShareLock on
     * the children.
     */
    tableOIDs =
        find_all_inheritors(RelationGetRelid(onerel), AccessShareLock, NULL);

It also seems possible for the call to vac_open_indexes() in
do_analyze_rel() to block.

    /*
     * Open all indexes of the relation, and see if there are any analyzable
     * columns in the indexes.  We do not analyze index columns if there was
     * an explicit column list in the ANALYZE command, however.  If we are
     * doing a recursive scan, we don't want to touch the parent's indexes at
     * all.
     */
    if (!inh)
        vac_open_indexes(onerel, AccessShareLock, &nindexes, &Irel);
    else
    {
        Irel = NULL;
        nindexes = 0;
    }

I think the most straightforward approach for fixing this is to add
skip-locked functionality in find_all_inheritors(),
find_inheritance_children(), and vac_open_indexes(), but I am curious
what others think.

Nathan