Re: pgsql: Fix "base" snapshot handling in logical decoding

From: Arseny Sher <a(dot)sher(at)postgrespro(dot)ru>
To: Arseny Sher <a(dot)sher(at)postgrespro(dot)ru>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, 2q-andrew(at)alvin(dot)alvh(dot)no-ip(dot)org
Subject: Re: pgsql: Fix "base" snapshot handling in logical decoding
Date: 2018-07-02 15:52:09
Message-ID: 87r2kliqja.fsf@ars-thinkpad
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers


Arseny Sher <a(dot)sher(at)postgrespro(dot)ru> writes:

> There is also one thing that puzzles me as I don't know much about
> vacuum internals. If I do plain VACUUM of pg_attribute in the test, it
> shouts "catalog is missing 1 attribute(s) for relid" error (which is
> quite expected), while with 'VACUUM FULL pg_attribute' the tuple is
> silently (and wrongly, with dropped column missing) decoded. Moreover,
> if I perform the test manually, and do 'VACUUM FULL;', sometimes test
> becomes useless -- that is, tuple is successfully decoded with all three
> columns, as though VACUUM was not actually executed. All this is without
> the main patch, of course. I think I will look into this soon.

So I have been jumping around this and learned a few curious things.

1) Test in its current shape sometimes doesn't fulfill its aim indeed --
that is, despite issued VACUUM the tuple is still decoded with all
three fields. This happens because during attempt to advance xmin
there is a good possiblity to encounter xl_running_xacts record logged
before our CHECKPOINT (they are logged each 15 seconds). We do not
try to advance xmin twice without client acknowledgment, so in this
case xmin will not be advanced far enough to allow vacuum prune entry
from pg_attribute.

2) This is not easy to notice because often (but not always) explicit
VACUUM is not needed at all: tuple is often pruned by microvacuum
(heap_page_prune_opts) right in the final decoding session. If we
hadn't bumped xmin far enough during previous get_changes, we do that
now, so microvacuum actually purges the entry. But if we were so
unfortunate that 1) extra xl_running_xacts was logged and 2)
microvacuum was in bad mood and didn't come, pg_attribute is not
vacuumed and test becomes useless. To make this bulletproof, in the
attached patch I doubled first get_changes: now there are two client
acks, so our VACUUM always does the job.

3) As a side note, answer to my question 'why do we get different errors
with VACUUM and VACUUM FULL' is the following. With VACUUM FULL, not
only old pg_attribute entry is pruned, but also xmin of new entry
with attisdropped=true is reset to frozen xid. This means that
decoding session (RelationBuildTupleDesc) actually sees 3 attributes,
and the fact that one of them is dropped doesn't embarrass this
function (apparently relnatts in pg_class is never decremented) --
we just go ahead and decode only live attributes.

--
Arseny Sher
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Attachment Content-Type Size
less_agressive_vacuum_in_oldest_xmin_test.patch text/x-diff 3.0 KB

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Stephen Frost 2018-07-02 16:05:47 Re: Tips on committing
Previous Message Michael Paquier 2018-07-02 13:23:52 pgsql: Add wait event for fsync of WAL segments

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Hernandez 2018-07-02 15:56:34 Re: [Proposal] Table-level Transparent Data Encryption (TDE) and Key Management Service (KMS)
Previous Message Tom Lane 2018-07-02 15:37:25 Re: branches_of_interest.txt