On Fri, Jul 06, 2018 at 03:37:57PM +0900, Kyotaro HORIGUCHI wrote: > I'm not so in favor of the word "reserve" in first place since it > doesn't seem intuitive for me, but "active" is already used for > the state where the connection with the peer is made. (The word > "reserve" may be misused since in the source code "reserve" is > used as "to reserve WAL", but used as "reserve a slot" in > documentation.)
That's the term used for now three releases, counting v11 in the pack, so I would not change that now. The concept of non-initialized slots is fuzzy as well as it could be attached to some other meta-data. So, chewing on all that, I would suggest the following error message as the attached patch and just move on: +ERROR: cannot move slot not reserving WAL -- Michael
diff --git a/contrib/test_decoding/expected/slot.out b/contrib/test_decoding/expected/slot.out index 21e9d56f73..cc3766e10e 100644 --- a/contrib/test_decoding/expected/slot.out +++ b/contrib/test_decoding/expected/slot.out @@ -131,3 +131,20 @@ SELECT pg_drop_replication_slot('regression_slot1'); ERROR: replication slot "regression_slot1" does not exist SELECT pg_drop_replication_slot('regression_slot2'); ERROR: replication slot "regression_slot2" does not exist +-- slot advance with physical slot, error with non-reserved slot +SELECT slot_name FROM pg_create_physical_replication_slot('regression_slot3'); + slot_name +------------------ + regression_slot3 +(1 row) + +SELECT pg_replication_slot_advance('regression_slot3', '0/0'); -- invalid LSN +ERROR: invalid target wal lsn +SELECT pg_replication_slot_advance('regression_slot3', '0/1'); -- error +ERROR: cannot move slot not reserving WAL +SELECT pg_drop_replication_slot('regression_slot3'); + pg_drop_replication_slot +-------------------------- + +(1 row) + diff --git a/contrib/test_decoding/sql/slot.sql b/contrib/test_decoding/sql/slot.sql index 706340c1d8..24cdf7155d 100644 --- a/contrib/test_decoding/sql/slot.sql +++ b/contrib/test_decoding/sql/slot.sql @@ -68,3 +68,9 @@ SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot1', 'test_ -- both should error as they should be dropped on error SELECT pg_drop_replication_slot('regression_slot1'); SELECT pg_drop_replication_slot('regression_slot2'); + +-- slot advance with physical slot, error with non-reserved slot +SELECT slot_name FROM pg_create_physical_replication_slot('regression_slot3'); +SELECT pg_replication_slot_advance('regression_slot3', '0/0'); -- invalid LSN +SELECT pg_replication_slot_advance('regression_slot3', '0/1'); -- error +SELECT pg_drop_replication_slot('regression_slot3'); diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 3ed9021c2f..4851bc2e24 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -9867,7 +9867,8 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx <entry></entry> <entry>The address (<literal>LSN</literal>) of oldest WAL which still might be required by the consumer of this slot and thus won't be - automatically removed during checkpoints. + automatically removed during checkpoints. <literal>NULL</literal> + if the <literal>LSN</literal> of this slot has never been reserved. </entry> </row> diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c index 2806e1076c..2f07cc37f5 100644 --- a/src/backend/replication/slotfuncs.c +++ b/src/backend/replication/slotfuncs.c @@ -483,6 +483,15 @@ pg_replication_slot_advance(PG_FUNCTION_ARGS) /* Acquire the slot so we "own" it */ ReplicationSlotAcquire(NameStr(*slotname), true); + /* A slot whose restart_lsn has never been reserved cannot be advanced */ + if (XLogRecPtrIsInvalid(MyReplicationSlot->data.restart_lsn)) + { + ReplicationSlotRelease(); + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot move slot not reserving WAL"))); + } + /* * Check if the slot is not moving backwards. Physical slots rely simply * on restart_lsn as a minimum point, while logical slots have confirmed
signature.asc
Description: PGP signature