Re: Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold <

On 2016-07-02 14:20:13 -0500, Kevin Grittner wrote:
> The possible fixes considered were these:
>
> (1) Always vacuum the heap before its related TOAST table.

I think that's clearly not ok from a cost perspective.

> (2) Same as (1) but only when old_snapshot_threshold >= 0.
> (3) Allow the special snapshot used for TOAST access to generate
> the "snapshot too old" error, so that the modified page from the
> pruning/vacuuming (along with other conditions) would cause that
> rather than something suggesting corrupt internal structure.
> (4) When looking to read a toasted value for a tuple past the
> early pruning horizon, if the value was not found consider it a
> "snapshot too old" error.

Doesn't solve the issue that a toast id might end up being reused.

> (5) Don't vacuum or prune a TOAST table except as part of the heap
> vacuum when early pruning is enabled.

That's pretty costly.

> (6) Don't allow early vacuuming/pruning of TOAST values except as
> part of the vacuuming of the related heap.

> It became evident pretty quickly that the HOT pruning of TOAST
> values should not do early cleanup, based on practical concerns of
> coordinating that with the heap cleanup for any of the above
> options. What's more, since we don't allow updating of tuples
> holding TOAST values, HOT pruning seems to be of dubious value on a
> TOAST table in general -- but removing that would be the matter for
> a separate patch.

I'm not following here. Sure, there'll be no HOT chains, but hot pruning
also releases space (though not item pointers) for dead tuples. And
that's fairly valuable in high-churn tables?

> Anyway, this patch includes a small hunk of code
> (two lines) to avoid early HOT pruning for TOAST tables.

I see it's only prohibiting the old_snapshot_threshold triggered
cleanup, good.

> For the vacuuming, option (6) seems a clear winner, and that is
> what this patch implements. A TOAST table can still be vacuumed on
> its own, but in that case it will not use old_snapshot_threshold to
> try to do any early cleanup.

> We were already normally vacuuming
> the TOAST table whenever we vacuumed the related heap; in such a
> case it uses the "oldestXmin" used for the heap to vacuum the TOAST
> table.

That's not the case. Autovacuum schedules main and toast tables
independently. Check the two collection loops in do_autovacuum:
/*
* On the first pass, we collect main tables to vacuum, and also the main
* table relid to TOAST relid mapping.
*/
while ((tuple = heap_getnext(relScan, ForwardScanDirection)) != NULL)
{
...
relation_needs_vacanalyze(relid, relopts, classForm, tabentry,
effective_multixact_freeze_max_age,
&dovacuum, &doanalyze, &wraparound);
...
/* relations that need work are added to table_oids */
if (dovacuum || doanalyze)
table_oids = lappend_oid(table_oids, relid);
}
...
/* second pass: check TOAST tables */
while ((tuple = heap_getnext(relScan, ForwardScanDirection)) != NULL)
{
...
relation_needs_vacanalyze(relid, relopts, classForm, tabentry,
effective_multixact_freeze_max_age,
&dovacuum, &doanalyze, &wraparound);

/* ignore analyze for toast tables */
if (dovacuum)
table_oids = lappend_oid(table_oids, relid);
}

So I don't think that approach still allows old snapshot related
cleanups for toast triggered vacuums? Is that an acceptable
restriction?

Greetings,

Andres Freund