Re: Non-reserved replication slots and slot advancing

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Andres Freund <andres@anarazel.de>, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>, pgsql-hackers@postgresql.org, petr@2ndquadrant.com, simon@2ndQuadrant.com
Date: 2018-07-10T00:54:28Z
Lists: pgsql-hackers

Attachments

On Mon, Jul 09, 2018 at 02:48:28PM -0400, Alvaro Herrera wrote:
> On 2018-Jul-09, Andres Freund wrote:
> > On 2018-07-09 15:48:33 +0900, Michael Paquier wrote:
> > > +	if (XLogRecPtrIsInvalid(MyReplicationSlot->data.restart_lsn))
> > > +	{
> > > +		ReplicationSlotRelease();
> > > +		ereport(ERROR,
> > > +				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> > > +				 errmsg("cannot move slot not reserving WAL")));
> > > +	}
> > > +
> > 
> > I don't like this error message. It's unclear what refers to exactly
> > what.  Nor is "move" a terminology used otherwise.  How about:
> > "cannot advance replication slot that has not previously reserved WAL"
> > or something similar?

"move" is used in another error message ten lines below.

> > 
> > Also, ERRCODE_FEATURE_NOT_SUPPORTED doesn't quite seem right. How about
> > ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE?
> 
> +1 to both of Andres' suggestions.

Those indeed sound better.  What do you think of the attached?
--
Michael

Commits

  1. Block replication slot advance for these not yet reserving WAL