Re: snapbuild woes

Petr Jelinek <petr.jelinek@2ndquadrant.com>

From: Petr Jelinek <petr.jelinek@2ndquadrant.com>
To: Andres Freund <andres@anarazel.de>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>, Erik Rijkers <er@xs4all.nl>, Robert Haas <robertmhaas@gmail.com>, Craig Ringer <craig@2ndquadrant.com>
Date: 2017-04-24T23:11:14Z
Lists: pgsql-hackers
On 25/04/17 00:59, Andres Freund wrote:
> Hi,
> 
> On 2017-04-15 05:18:49 +0200, Petr Jelinek wrote:
>> Hi, here is updated patch (details inline).
> 
> I'm not yet all that happy, sorry:
> 
> 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.
> - A later commit introduces need_full_snapshot as a
>   CreateInitDecodingContext, but you don't use it, not here.  That seems
>   wrong.

Ah yeah looks like that optimization is useful even here.

> - I remain unhappy with the handling of the reset of effective_xmin in
>   FreeDecodingContext(). What if we ERROR/FATAL out before that happens?
> 

Oh your problem was that I did it in FreeDecodingContext() instead of
slot release, that I didn't get, yeah sure it's possibly better place.

> What do you think about something like the attached?  I've not yet
> tested it any way except running the regression tests.
> 

> -	if (!already_locked)
> -		LWLockAcquire(ReplicationSlotControlLock, LW_SHARED);
> +	LWLockAcquire(ReplicationSlotControlLock, LW_SHARED);

Don't really understand this change much, but otherwise the patch looks
good to me.

-- 
  Petr Jelinek                  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services


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.