Re: BUG #13750: Autovacuum slows down with large numbers of tables. More workers makes it slower.

On Tue, 15 Mar 2016 14:28:16 -0700
David Gould <daveg@sonic.net> wrote:

> On Tue, 15 Mar 2016 17:40:26 -0300
> Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>
> > David Gould wrote:
> > > On Mon, 29 Feb 2016 18:33:50 -0300
> > > Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> > >
> > > > Hi David, did you ever post an updated version of this patch?
> > >
> > > No. Let me fix that now. I've attached my current revision of the patch
> > > based on master. This version is significantly better than the original
> > > version and resolves multiple issues:
> >
> > Thanks for the patch.  I spent some time studying it and testing it.  I
> > think there's a minor bug in this latest version: the "continue" when
> > the "for" loop exits because of all relids being less than
> > highest_oid_claimed should be a "break" instead.  Otherwise the worker
> > will uselessly loop over the whole list of OIDs only to find each time
> > that it has nothing to do.
>
> Good catch. Thanks!

Actually, looking at this more closely, I don't see the problem.
I'm assuming you meant the continue after "if (relid ==InvalidOid)":

    /*
     * 2. While there are items that have not been visited...
     */
    for(oid_idx = 0; oid_idx < numoids; )
    {
 ...
        /*
         * 4a. Skip past the highest_oid_claimed to find the next oid to work on.
         */
        relid = InvalidOid;
        while (relid <= highest_oid_claimed && oid_idx < numoids)
            relid = table_oids[oid_idx++];
        if (relid <= highest_oid_claimed)
            relid = InvalidOid;
        /* 4b. Claim the chosen table by storing its oid in shared memory. */
        MyWorkerInfo->wi_tableoid = relid;

        LWLockRelease(AutovacuumLock);

        /* If we reached the end of the list there is nothing to do. */
        if (relid == InvalidOid)
            continue;
 ...
    }

At the continue oid_idx will have been incremented by:

        while (relid <= highest_oid_claimed && oid_idx < numoids)
            relid = table_oids[oid_idx++];

to be equal to numoids so the for loop will exit immediately as the loop
condition is "oid_idx < numoids".

"break" would work as well, but basically there is only one reason to exit
the for loop, exhausting the list of oids, so continue seemed preferable.

Or am I missing something?

-dg

--
David Gould              510 282 0869         daveg@sonic.net
If simplicity worked, the world would be overrun with insects.