On 2017-06-09 09:25:34 +0200, Antonin Houska wrote: > Andres Freund <and...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
