Hannu Krosing <hannu@skype.net> writes:
> On T, 2005-08-16 at 18:26 -0400, Tom Lane wrote:
>> Some specific concerns:
>>
>> * Given that VACUUM ANALYZE does create new output tuples stamped with
>> its xid, I'm unclear on what happens in pg_statistic with this code in
>> place.
> Actually any VACUUM, not only VACUUM ANALYSE, updates pg_class at the end.
> That's why I exclude only one of the transactions of the VACUUM command, and that
> transaction does not create any new tuples, it only removes old ones.
vac_update_relstats isn't the issue because it doesn't create a new
tuple. I was concerned about ANALYZE --- but since that's done in a
separate transaction that's not marked inVacuum, it's not at risk. So
OK, that's all right.
>> * If the vacuum xact is older than what others think is the global xmin,
>> it could have problems with other vacuums removing tuples it should
>> still be able to see (presumably only in the system catalogs, so maybe
>> this isn't an issue, but I'm unsure).
> The cleanup transaction does no lookups in system catalogs.
Oh? It certainly has to open relations and indexes. I think that all
of that stuff may be done with SnapshotNow, rather than an xmin-related
snap, but it's still nervous-making.
>> A related scenario that I don't
>> think can be dismissed is someone truncating off part of pg_subtrans or
>> pg_multixact that the vacuum still needs.
> In my patch I specifically exclude TruncateSUBTRANS from using the
> inVacuum flag
You missed vac_truncate_clog, though.
> So I think that both your concerns expressed here are _already_
> addressed by the latest patch in:
> http://archives.postgresql.org/pgsql-patches/2005-07/msg00086.php
I have to admit that in my earlier message, I was looking at the version
of the patch that Bruce had on his patch page --- which I now see was not
the latest. The idea of making GetOldestXmin only conditionally ignore
vacuums certainly makes it a lot safer.
> Please check the actual patch and advise if anything is still missing.
There's still a fair amount of breakage in this patch --- eg, in the
VACUUM FULL case it manages to invoke *both* full_vacuum_rel and
lazy_vacuum_rel --- but I think it can probably be made to work.
I'll take another pass at it.
regards, tom lane
