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(at)sonic(dot)net> wrote:

> On Tue, 15 Mar 2016 17:40:26 -0300
> Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote:
>
> > David Gould wrote:
> > > On Mon, 29 Feb 2016 18:33:50 -0300
> > > Alvaro Herrera <alvherre(at)2ndquadrant(dot)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(at)sonic(dot)net
If simplicity worked, the world would be overrun with insects.