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

Arseny Sher <a.sher@postgrespro.ru>

From: Arseny Sher <a.sher@postgrespro.ru>
To: Arseny Sher <a.sher@postgrespro.ru>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Alvaro Herrera <alvherre@2ndquadrant.com>, pgsql-hackers@lists.postgresql.org, 2q-andrew@alvin.alvh.no-ip.org
Date: 2018-07-02T15:52:09Z
Lists: pgsql-hackers

Attachments

Arseny Sher <a.sher@postgrespro.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

Commits

  1. Reduce cost of test_decoding's new oldest_xmin test

  2. Fix "base" snapshot handling in logical decoding