Date: Tue, 15 Sep 2020 18:33:16 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Michael Paquier <michael@paquier.xyz>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: Gripes about walsender command processing
Message-ID: <20200915213316.GA12416@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <1347966.1600195735@sss.pgh.pa.us>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On 2020-Sep-15, Tom Lane wrote:

> [ blink ... ]  So why is it that DropReplicationSlot does
> 
> 	SetQueryCompletion(&qc, CMDTAG_DROP_REPLICATION_SLOT, 0);
> 	EndCommand(&qc, DestRemote, false);
> 
> when the caller will immediately after that do
> 
> 	SetQueryCompletion(&qc, CMDTAG_SELECT, 0);
> 	EndCommand(&qc, DestRemote, true);
> 
> StartLogicalReplication has a similar weirdness.
> The more I look at this code, the more broken it seems.

I overlooked this in 2f9661311b83.  From this perspective, I agree it
looks wrong.  We still have to send *some* completion tag (the 'C'
message), but maybe we can invent a separate entry point in dest.c for
that -- EndReplicationCommand() or some such -- that takes values from a
separate enum?

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services