On Mon, Nov 12, 2012 at 5:14 PM, Andrew Dunstan <andrew@dunslane.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