expose confirmed_flush for replication slots

Started by Marko Tiikkajaover 10 years ago4 messages
#1Marko Tiikkaja
marko@joh.to
1 attachment(s)

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
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index e82a53a..ffaf451 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -674,7 +674,8 @@ CREATE VIEW pg_replication_slots AS
             L.active_pid,
             L.xmin,
             L.catalog_xmin,
-            L.restart_lsn
+            L.restart_lsn,
+            L.confirmed_flush_lsn
     FROM pg_get_replication_slots() AS L
             LEFT JOIN pg_database D ON (L.datoid = D.oid);
 
diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index 9a2793f..90d75ce 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -206,6 +206,7 @@ pg_get_replication_slots(PG_FUNCTION_ARGS)
 		TransactionId xmin;
 		TransactionId catalog_xmin;
 		XLogRecPtr	restart_lsn;
+		XLogRecPtr	confirmed_flush_lsn;
 		pid_t		active_pid;
 		Oid			database;
 		NameData	slot_name;
@@ -224,6 +225,7 @@ pg_get_replication_slots(PG_FUNCTION_ARGS)
 			catalog_xmin = slot->data.catalog_xmin;
 			database = slot->data.database;
 			restart_lsn = slot->data.restart_lsn;
+			confirmed_flush_lsn = slot->data.confirmed_flush;
 			namecpy(&slot_name, &slot->data.name);
 			namecpy(&plugin, &slot->data.plugin);
 
@@ -273,6 +275,11 @@ pg_get_replication_slots(PG_FUNCTION_ARGS)
 		else
 			nulls[i++] = true;
 
+		if (confirmed_flush_lsn != InvalidTransactionId)
+			values[i++] = LSNGetDatum(confirmed_flush_lsn);
+		else
+			nulls[i++] = true;
+
 		tuplestore_putvalues(tupstore, tupdesc, values, nulls);
 	}
 
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 6fd1278..aee82d2 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -5183,7 +5183,7 @@ DATA(insert OID = 3779 (  pg_create_physical_replication_slot PGNSP PGUID 12 1 0
 DESCR("create a physical replication slot");
 DATA(insert OID = 3780 (  pg_drop_replication_slot PGNSP PGUID 12 1 0 0 0 f f f f f f v 1 0 2278 "19" _null_ _null_ _null_ _null_ _null_ pg_drop_replication_slot _null_ _null_ _null_ ));
 DESCR("drop a replication slot");
-DATA(insert OID = 3781 (  pg_get_replication_slots	PGNSP PGUID 12 1 10 0 0 f f f f f t s 0 0 2249 "" "{19,19,25,26,16,23,28,28,3220}" "{o,o,o,o,o,o,o,o,o}" "{slot_name,plugin,slot_type,datoid,active,active_pid,xmin,catalog_xmin,restart_lsn}" _null_ _null_ pg_get_replication_slots _null_ _null_ _null_ ));
+DATA(insert OID = 3781 (  pg_get_replication_slots	PGNSP PGUID 12 1 10 0 0 f f f f f t s 0 0 2249 "" "{19,19,25,26,16,23,28,28,3220,3220}" "{o,o,o,o,o,o,o,o,o,o}" "{slot_name,plugin,slot_type,datoid,active,active_pid,xmin,catalog_xmin,restart_lsn,confirmed_flush_lsn}" _null_ _null_ pg_get_replication_slots _null_ _null_ _null_ ));
 DESCR("information about replication slots currently in use");
 DATA(insert OID = 3786 (  pg_create_logical_replication_slot PGNSP PGUID 12 1 0 0 0 f f f f f f v 2 0 2249 "19 19" "{19,19,25,3220}" "{i,i,o,o}" "{slot_name,plugin,slot_name,xlog_position}" _null_ _null_ pg_create_logical_replication_slot _null_ _null_ _null_ ));
 DESCR("set up a logical replication slot");
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index cd53375..63cd323 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1416,8 +1416,9 @@ pg_replication_slots| SELECT l.slot_name,
     l.active_pid,
     l.xmin,
     l.catalog_xmin,
-    l.restart_lsn
-   FROM (pg_get_replication_slots() l(slot_name, plugin, slot_type, datoid, active, active_pid, xmin, catalog_xmin, restart_lsn)
+    l.restart_lsn,
+    l.confirmed_flush_lsn
+   FROM (pg_get_replication_slots() l(slot_name, plugin, slot_type, datoid, active, active_pid, xmin, catalog_xmin, restart_lsn, confirmed_flush_lsn)
      LEFT JOIN pg_database d ON ((l.datoid = d.oid)));
 pg_roles| SELECT pg_authid.rolname,
     pg_authid.rolsuper,
#2Marko Tiikkaja
marko@joh.to
In reply to: Marko Tiikkaja (#1)
1 attachment(s)
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
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index e82a53a..ffaf451 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -674,7 +674,8 @@ CREATE VIEW pg_replication_slots AS
             L.active_pid,
             L.xmin,
             L.catalog_xmin,
-            L.restart_lsn
+            L.restart_lsn,
+            L.confirmed_flush_lsn
     FROM pg_get_replication_slots() AS L
             LEFT JOIN pg_database D ON (L.datoid = D.oid);
 
diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index 9a2793f..ddbf8c4 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -158,7 +158,7 @@ pg_drop_replication_slot(PG_FUNCTION_ARGS)
 Datum
 pg_get_replication_slots(PG_FUNCTION_ARGS)
 {
-#define PG_GET_REPLICATION_SLOTS_COLS 9
+#define PG_GET_REPLICATION_SLOTS_COLS 10
 	ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
 	TupleDesc	tupdesc;
 	Tuplestorestate *tupstore;
@@ -206,6 +206,7 @@ pg_get_replication_slots(PG_FUNCTION_ARGS)
 		TransactionId xmin;
 		TransactionId catalog_xmin;
 		XLogRecPtr	restart_lsn;
+		XLogRecPtr	confirmed_flush_lsn;
 		pid_t		active_pid;
 		Oid			database;
 		NameData	slot_name;
@@ -224,6 +225,7 @@ pg_get_replication_slots(PG_FUNCTION_ARGS)
 			catalog_xmin = slot->data.catalog_xmin;
 			database = slot->data.database;
 			restart_lsn = slot->data.restart_lsn;
+			confirmed_flush_lsn = slot->data.confirmed_flush;
 			namecpy(&slot_name, &slot->data.name);
 			namecpy(&plugin, &slot->data.plugin);
 
@@ -273,6 +275,11 @@ pg_get_replication_slots(PG_FUNCTION_ARGS)
 		else
 			nulls[i++] = true;
 
+		if (confirmed_flush_lsn != InvalidTransactionId)
+			values[i++] = LSNGetDatum(confirmed_flush_lsn);
+		else
+			nulls[i++] = true;
+
 		tuplestore_putvalues(tupstore, tupdesc, values, nulls);
 	}
 
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 6fd1278..aee82d2 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -5183,7 +5183,7 @@ DATA(insert OID = 3779 (  pg_create_physical_replication_slot PGNSP PGUID 12 1 0
 DESCR("create a physical replication slot");
 DATA(insert OID = 3780 (  pg_drop_replication_slot PGNSP PGUID 12 1 0 0 0 f f f f f f v 1 0 2278 "19" _null_ _null_ _null_ _null_ _null_ pg_drop_replication_slot _null_ _null_ _null_ ));
 DESCR("drop a replication slot");
-DATA(insert OID = 3781 (  pg_get_replication_slots	PGNSP PGUID 12 1 10 0 0 f f f f f t s 0 0 2249 "" "{19,19,25,26,16,23,28,28,3220}" "{o,o,o,o,o,o,o,o,o}" "{slot_name,plugin,slot_type,datoid,active,active_pid,xmin,catalog_xmin,restart_lsn}" _null_ _null_ pg_get_replication_slots _null_ _null_ _null_ ));
+DATA(insert OID = 3781 (  pg_get_replication_slots	PGNSP PGUID 12 1 10 0 0 f f f f f t s 0 0 2249 "" "{19,19,25,26,16,23,28,28,3220,3220}" "{o,o,o,o,o,o,o,o,o,o}" "{slot_name,plugin,slot_type,datoid,active,active_pid,xmin,catalog_xmin,restart_lsn,confirmed_flush_lsn}" _null_ _null_ pg_get_replication_slots _null_ _null_ _null_ ));
 DESCR("information about replication slots currently in use");
 DATA(insert OID = 3786 (  pg_create_logical_replication_slot PGNSP PGUID 12 1 0 0 0 f f f f f f v 2 0 2249 "19 19" "{19,19,25,3220}" "{i,i,o,o}" "{slot_name,plugin,slot_name,xlog_position}" _null_ _null_ pg_create_logical_replication_slot _null_ _null_ _null_ ));
 DESCR("set up a logical replication slot");
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index cd53375..63cd323 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1416,8 +1416,9 @@ pg_replication_slots| SELECT l.slot_name,
     l.active_pid,
     l.xmin,
     l.catalog_xmin,
-    l.restart_lsn
-   FROM (pg_get_replication_slots() l(slot_name, plugin, slot_type, datoid, active, active_pid, xmin, catalog_xmin, restart_lsn)
+    l.restart_lsn,
+    l.confirmed_flush_lsn
+   FROM (pg_get_replication_slots() l(slot_name, plugin, slot_type, datoid, active, active_pid, xmin, catalog_xmin, restart_lsn, confirmed_flush_lsn)
      LEFT JOIN pg_database d ON ((l.datoid = d.oid)));
 pg_roles| SELECT pg_authid.rolname,
     pg_authid.rolsuper,
#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