Re: PATCH to allow concurrent VACUUMs to not lock each

On E, 2005-07-04 at 10:24 +0300, Hannu Krosing wrote:
> On P, 2005-07-03 at 12:19 -0400, Tom Lane wrote:
> > Hannu Krosing <hannu(at)skype(dot)net> writes:
> > > Ok, this is a new version of the vacuum patch with the following changes
> > > following some suggestions in this thread.
> >
> > The more I look at this, the uglier it looks ... and I still haven't
> > seen any convincing demonstration that it *works*, ie doesn't have
> > bad side-effects on the transaction-is-in-progress logic.

Ok, I changed GetOldestXmin() to use proc->inVacuum only when
determining the oldest visible xid for vacuum and index (i.e. which
tuples are safe to delete and which tuples there is no need to index).

The third use on GetOldestXmin() in xlog.c is changed to use old
GetOldestXmin() logic.

My reasoning for why the patch should work is as follows:

1) the only transaction during which inVacuum is set is the 2nd
transaction (of originally 3, now 4) of lazy VACUUM, which does simple
heap scanning and old tuple removal (lazy_vacuum_rel()), and does no
externally visible changes to the data. It only removes tuples which are
already invisible to all running transactions.

2) That transaction never deletes, updates or inserts any tuples on it
own.

3) As it can't add any tuples or change any existing tuples to have its
xid as either xmin or xmax, it already does run logically "outside of
transactions".

4) The only use made of of proc->inVacuum is when determining which
tuples are safe to remove (in vacuum.c) or not worth indexing (in
index.c) and thus can't affect anything else.

I can easily demonstrate that it "works" in the sense that it allows
several concurrent vacuums to clean out old tuples, and I have thus far
been unable to construct the counterexample where it does anything bad.

Could you tell me which part of my reasoning is wrong or what else do I
overlook.

--
Hannu Krosing <hannu(at)tm(dot)ee>