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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
