Re: vacuumlo - use a cursor

On Mon, Nov 12, 2012 at 5:14 PM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
> vacuumlo is rather simpleminded about dealing with the list of LOs to be
> removed - it just fetches them as a straight resultset. For one of my our
> this resulted in an out of memory condition.

Wow, they must have had a ton of LOs. With about 2M entries to pull
from vacuum_l, I observed unpatched vacuumlo using only about 45MB
RES. Still, the idea of using a cursor for the main loop seems like a
reasonable idea.

> The attached patch tries to
> remedy that by using a cursor instead. If this is wanted I will add it to
> the next commitfest. The actualy changes are very small - most of the patch
> is indentation changes due to the introduction of an extra loop.

I had some time to review this, some comments about the patch:

1. I see this new compiler warning:

vacuumlo.c: In function ‘vacuumlo’:
vacuumlo.c:306:5: warning: format ‘%lld’ expects argument of type
‘long long int’, but argument 4 has type ‘long int’ [-Wformat]

2. It looks like the the patch causes all the intermediate result-sets
fetched from the cursor to be leaked, rather negating its stated
purpose ;-) The PQclear() call should be moved up into the main loop.
With this fixed, I confirmed that vacuumlo now consumes a negligible
amount of memory when chewing through millions of entries.

3. A few extra trailing whitespaces were added.

4. The FETCH FORWARD count comes from transaction_limit. That seems
like a good-enough guesstimate, but maybe a comment could be added to
rationalize?

Some suggested changes attached with v2 patch (all except #4).

Josh