Re: pg_amcheck contrib application

> On Mar 31, 2021, at 10:31 AM, Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com> wrote:
>
>
>
>> On Mar 31, 2021, at 10:11 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>
>> On Wed, Mar 31, 2021 at 12:34 AM Mark Dilger
>> <mark(dot)dilger(at)enterprisedb(dot)com> wrote:
>>> I'm not looking at the old VACUUM FULL code, but my assumption is that if the xvac code were resurrected, then when a tuple is moved off by a VACUUM FULL, the old tuple and associated toast cannot be pruned until concurrent transactions end. So, if amcheck is running more-or-less concurrently with the VACUUM FULL and has a snapshot xmin no newer than the xid of the VACUUM FULL's xid, it can check the toast associated with the moved off tuple after the VACUUM FULL commits. If instead the VACUUM FULL xid was older than amcheck's xmin, then the toast is in danger of being vacuumed away. So the logic in verify_heapam would need to change to think about this distinction. We don't have to concern ourselves about that, because VACUUM FULL cannot be running, and so the xid for it must be older than our xmin, and hence the toast is unconditionally not safe to check.
>>>
>>> I'm changing the comments back to how you had them, but I'd like to know why my reasoning is wrong.
>>
>> Let's start by figuring out *whether* your reasoning is wrong. My
>> assumption was that old-style VACUUM FULL would move tuples without
>> retoasting. That is, if we decided to move a tuple from page 2 of the
>> main table to page 1, we would just write the tuple into page 1,
>> marking it moved-in, and on page 2 we would mark it moved-off. And
>> that we would not examine or follow any TOAST pointers at all, so
>> whatever TOAST entries existed would be shared between the two tuples.
>> One tuple or the other would eventually die, depending on whether xvac
>> went on to commit or abort, but either way the TOAST doesn't need
>> updating because there's always exactly 1 remaining tuple using
>> pointers to those TOAST values.
>>
>> Your assumption seems to be the opposite, that the TOASTed values
>> would be retoasted as part of VF. If that is true, then your idea is
>> right.
>>
>> Do you agree with this analysis? If so, we can check the code and find
>> out which way it actually worked.
>
> Actually, that makes a lot of sense without even looking at the old code. I was implicitly assuming that the toast table was undergoing a VF also, and that the toast pointers in the main table tuples would be updated to point to the new location, so we'd be unable to follow the pointers to the old location without danger of the old location entries being vacuumed away. But if the main table tuples get moved while keeping their toast pointers unaltered, then you don't have to worry about that, although you do have to worry that a VF of the main table doesn't help so much with toast table bloat.
>
> We're only discussing this in order to craft the right comment for a bit of code with respect to a hypothetical situation in which VF gets resurrected, so I'm not sure this should be top priority, but I'm curious enough now to go read the old code....

Well, that's annoying. The documentation of postgres 8.2 for vacuum full [1] says,

Selects "full" vacuum, which may reclaim more space, but takes much longer and exclusively locks the table.

I read "exclusively locks" as meaning it takes an ExclusiveLock, but the code shows that it takes an AccessExclusiveLock. I think the docs are pretty misleading here, though I understand that grammatically it is hard to say "accessively exclusively locks" or such. But a part of my analysis was based on the reasoning that if VF only takes an ExclusiveLock, then there must be concurrent readers possible. VF went away long enough ago that I had forgotten exactly how inconvenient it was.

[1] https://www.postgresql.org/docs/8.2/sql-vacuum.html

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company