Logical decoding of sequence advances, part II

From: Craig Ringer <craig(at)2ndquadrant(dot)com>
To: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Cc: Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com>, Andres Freund <andres(at)anarazel(dot)de>
Subject: Logical decoding of sequence advances, part II
Date: 2016-08-22 03:13:31
Message-ID: CAMsr+YFtoGcLfs4z3sjMX1yLxWE37MJcJ+NkRqPwaJjr_LE-Xg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi all

My earlier efforts at logical decoding of sequence advances were too
simplistic[1], falling afoul of issues with sequences being both
transactional and not transactional depending on whether the sequence is
created in the current xact or not.

TL;DR of solution:

* extend SeqTableData and xl_seq_rec with xid
* set xid in SeqTableData during DefineSequence()
* if SeqTableData set, write it in xl_seq_rec when writing xlog
* assign sequence update to specified xid's reorder buffer in decoding if
xid set, otherwise decode immediately

The problem:

If the sequence is created in the current xact (i.e. uncommitted) we have
to add the sequence updates to that xact to be replayed only if it commits.
The sequence is visible only to the toplevel xact that created the sequence
so advances of it can only come from that xact and its children. The actual
CREATE SEQUENCE is presumed to be handled separately by an event trigger or
similar.

If the new sequence is committed we must replay sequence advances
immediately and non-transactionally to ensure they're not lost due to xact
rollback or replayed in the wrong order due to xact commit order.

If the sequence is ALTERed in a way that changes pg_class that's event
triggers' job and sequence decoding doesn't care. If it's ALTERed in a way
that changes Form_pg_sequence we replay the change immediately, using the
last committed snapshot to get the sequence details, so the change will
take immediate effect and is retained whether or not any pg_class changes
are committed. This reflects how it happens on the upstream.

Planned solution:

Extend xl_seq_rec with a created_in_xid TransactionId field. If
created_in_xid != InvalidTransactionId, logical decoding associates the
sequence advance with the given toplevel xact and adds it to the reorder
buffer instead of immediately invoking the sequence decoding callback. The
decoding callback then gets invoked during ReorderBufferCommit processing
at the appropriate time, like any other transactional change.

To determine whether to log an xid for the sequence advance we need some
backend local state to determine whether the sequence is new in this xact.
Handily we already have one, the seqhashtab of SeqTableData in sequence.c,
just where it's needed. So all that's needed is to add a TransactionId
field that we set if we created that sequence in this session. If it's set
we test it for TransactionIsInProgress() when xlog'ing a sequence advance;
if it is, log that xid. If not in progress, clear the xid in SeqTableData
entry so we don't check again.

Another approach would be, during decoding, to look up the relfilenode of
the sequence to get the sequence oid and do a pg_class lookup. Check to see
whether xmin is part of an in-progress xact. If so, add the sequence
advance to that xact's reorder buffer, otherwise decode it immediately. The
problem is that

(a) I think we lack relfilenode-to-oid mapping information at
decoding-time. RelidByRelfilenode() needs a snapshot and is invoked
during ReorderBufferCommit(). We have make the transactional vs
nontransactional decision in LogicalDecodingProcessRecord() when I'm pretty
sure we don't have a snapshot.

(b) It also has issues with ALTER TRANSACTION. We must replay decoded xact
updates immediately even if some in-flight xact has modified the pg_class
entry for the sequence. So we can't just check whether the xmin is one of
our xact's (sub)xids, we must also check whether some older tuple for the
same sequence oid has a corresponding xmax and keep walking backwards until
we determine whether we originally CREATEd the sequence in this xact or
only ALTERed it.

So yeah. I think extending SeqTableData and xl_seq_rec with xid is the way
to go. Objections?

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

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Craig Ringer 2016-08-22 03:17:25 Re: replication slots replicated to standbys?
Previous Message Michael Paquier 2016-08-22 02:49:02 Re: replication slots replicated to standbys?