Re: snapbuild woes

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: Antonin Houska <ah@cybertec.at>
Cc: pgsql-hackers@postgresql.org
Date: 2017-08-06T21:37:31Z
Lists: pgsql-hackers
On 2017-06-09 09:25:34 +0200, Antonin Houska wrote:
> Andres Freund <andres@anarazel.de> wrote:
> 
> > Looking at 0001:
> > - GetOldestSafeDecodingTransactionId() only guarantees to return an xid
> >   safe for decoding (note how procArray->replication_slot_catalog_xmin
> >   is checked), not one for the initial snapshot - so afaics this whole
> >   exercise doesn't guarantee much so far.
> 
> I happen to use CreateInitDecodingContext() in an extension, so I had to think
> what the new argumen exactly means (as for the incompatibility between PG
> 9.6.2 and 9.6.3, I suppose preprocessor directives can handle it).
> 
> One thing I'm failing to understand is: if TRUE is passed for
> need_full_snapshot, then slot->effective_xmin receives the result of
> 
>     GetOldestSafeDecodingTransactionId(need_full_snapshot)
> 
> but this does include "catalog xmin".
> 
> If the value returned is determined by an existing slot which has valid
> effective_catalog_xmin and invalid effective_xmin (i.e. that slot only
> protects catalog tables from VACUUM but not the regular ones), then the new
> slot will think it (i.e. the new slot) protects even non-catalog tables, but
> that's no true.
> 
> Shouldn't the xmin_horizon be computed by this call instead?
> 
>     GetOldestSafeDecodingTransactionId(!need_full_snapshot);
> 
> (If so, I think "considerCatalog" argument name would be clearer than
> "catalogOnly".)

Good catch. Pushing a fix. Afaict it's luckily inconsequential atm
because fo the way we wait for concurrent snapshots when creating a
slot. But it obviously nevertheless needs tobe fixed.

Greetings,

Andres Freund


Commits

  1. Fix thinko introduced in 2bef06d516460 et al.

  2. Avoid superfluous work for commits during logical slot creation.

  3. Fix race condition leading to hanging logical slot creation.

  4. Don't use on-disk snapshots for exported logical decoding snapshot.

  5. Preserve required !catalog tuples while computing initial decoding snapshot.