Hi,
On 24/11/17 07:41, Craig Ringer wrote:
> On 24 November 2017 at 13:44, Nikhil Sontakke <nikhils@2ndquadrant.com
>
>
> > How practical is adding a lock class?
>
> Am open to suggestions. This looks like it could work decently.
>
>
> It looks amazingly simple from here. Which probably means there's more
> to it that I haven't seen yet. I could use advice from someone who knows
> the locking subsystem better.
>
Hmm, I don't like the interaction that would have with ROLLBACK, meaning
that ROLLBACK has to wait for decoding to finish which may take longer
than the transaction took itself (given potential network calls, it's
practically unbounded time).
I also think that if we'll want to add streaming of transactions in the
future, we'll face similar problem and the locking approach will not
work there as the transaction may still be locked by the owning backend
while we are decoding it.
From my perspective this patch changes the assumption in
HeapTupleSatisfiesVacuum() that changes done by aborted transaction
can't be seen by anybody else. That's clearly not true here as the
decoding can see it. So perhaps better approach would be to not return
HEAPTUPLE_DEAD if the transaction id is newer than the OldestXmin (same
logic we use for deleted tuples of committed transactions) in the
HeapTupleSatisfiesVacuum() even for aborted transactions. I also briefly
checked HOT pruning and AFAICS the normal HOT pruning (the one not
called by vacuum) also uses the xmin as authoritative even for aborted
txes so nothing needs to be done there probably.
In case we are worried that this affects cleanups of for example large
aborted COPY transactions and we think it's worth worrying about then we
could limit the new OldestXmin based logic only to catalog tuples as
those are the only ones we need available in decoding.
-- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services