expose confirmed_flush for replication slots

Started by Marko Tiikkajaalmost 11 years ago4 messageshackers
Jump to latest
#1Marko Tiikkaja
marko@joh.to

Hi,

I had some trouble today with a misbehaving logical replication client
which had confirmed a flush of an LSN far into the future. Debugging it
was a bit of a pain for a number of reasons, but I think the most
important one was that confirmed_flush isn't exposed in
pg_stat_replication_slots. Attached patch exposes it, as
confirmed_flush_lsn (to make it look like restart_lsn; not sure whether
we should just keep the internal name or not).

Adding this one to the next commit fest, but any feedback welcome in the
meanwhile.

.m

Attachments:

confirmed_flush.patchtext/plain; charset=UTF-8; name=confirmed_flush.patch; x-mac-creator=0; x-mac-type=0Download+13-4
#2Marko Tiikkaja
marko@joh.to
In reply to: Marko Tiikkaja (#1)
Re: expose confirmed_flush for replication slots

On 2015-07-08 14:57, I wrote:

Adding this one to the next commit fest, but any feedback welcome in the
meanwhile.

Forgot to change PG_GET_REPLICATION_SLOTS_COLS; fixed in the attached patch.

.m

Attachments:

confirmed_flushv2.patchtext/plain; charset=UTF-8; name=confirmed_flushv2.patch; x-mac-creator=0; x-mac-type=0Download+14-5
#3Andres Freund
andres@anarazel.de
In reply to: Marko Tiikkaja (#2)
Re: expose confirmed_flush for replication slots

On 2015-07-08 15:01:15 +0300, Marko Tiikkaja wrote:

+		if (confirmed_flush_lsn != InvalidTransactionId)
+			values[i++] = LSNGetDatum(confirmed_flush_lsn);
+		else
+			nulls[i++] = true;
+

Hm. That comparison is using the wrong datatype, but it appears you only
copied my earlier mistake... Fixed back to 9.4 and in your patch.

Other notes:

* you missed to touch test_decoding's regression test output files.
* None of the docs were touched. catalogs.sgml definitely needs docs
about the new columns, and I see no reason to leave the examples
elsewhere stale.

Fixed those and committed it. Thanks for the patch!

- Andres

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Marko Tiikkaja
marko@joh.to
In reply to: Andres Freund (#3)
Re: expose confirmed_flush for replication slots

On 8/10/15 1:29 PM, Andres Freund wrote:

On 2015-07-08 15:01:15 +0300, Marko Tiikkaja wrote:

+		if (confirmed_flush_lsn != InvalidTransactionId)
+			values[i++] = LSNGetDatum(confirmed_flush_lsn);
+		else
+			nulls[i++] = true;
+

Hm. That comparison is using the wrong datatype, but it appears you only
copied my earlier mistake... Fixed back to 9.4 and in your patch.

Other notes:

* you missed to touch test_decoding's regression test output files.
* None of the docs were touched. catalogs.sgml definitely needs docs
about the new columns, and I see no reason to leave the examples
elsewhere stale.

Yeah. I should've grepped around a bit more. Sorry about that.

Fixed those and committed it. Thanks for the patch!

Thank you very much!

.m

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers