Re: More protocol.h replacements this time into walsender.c

From: "Euler Taveira" <euler(at)eulerto(dot)com>
To: "Nathan Bossart" <nathandbossart(at)gmail(dot)com>, "Jacob Champion" <jacob(dot)champion(at)enterprisedb(dot)com>
Cc: Álvaro Herrera <alvherre(at)kurilemu(dot)de>, "Dave Cramer" <davecramer(at)gmail(dot)com>, "PostgreSQL Hackers" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: More protocol.h replacements this time into walsender.c
Date: 2025-08-06 00:18:34
Message-ID: 797cad77-647b-42c4-89b8-fd5bf8a8fd10@app.fastmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Aug 5, 2025, at 4:58 PM, Nathan Bossart wrote:
> Here is an updated patch that includes 1) added uses of PqMsg_* macros, 2)
> new PqReplMsg_* macros, and 3) new PqBackupMsg_* macros. Thoughts?
>

- * 'd' means a standby reply wrapped in a CopyData packet.
+ * CopyData means a standby reply wrapped in a CopyData
+ * packet.
*/

Shouldn't it be PqMsg_CopyData instead of CopyData (first reference)?

The function LogicalParallelApplyLoop() has

/*
* The first byte of messages sent from leader apply worker to
* parallel apply workers can only be 'w'.
*/
c = pq_getmsgbyte(&s);
if (c != 'w')
elog(ERROR, "unexpected message \"%c\"", c);

it needs to be adjusted.

The function XLogWalRcvProcessMsg() has

switch (type)
{
case 'w': /* WAL records */
{
StringInfoData incoming_message;

...

case 'k': /* Keepalive */
{
StringInfoData incoming_message;

it also needs to be adjusted.

There is also some references into receivelog.c. The functions sendFeedback()
and HandleCopyStream() contain some references to be replaced. There are other
functions too that refers to these replication codes on comments. It seems a
good idea to replace these references too.

There are also some references in pg_recvlogical.c. The functions
sendFeedback() and StreamLogicalLog() contain some references to be replaced.

Alvaro already pointed out the other cases in the pg_basebackup.c file.

May I suggest that you put these code before authentication codes? It seems
natural to have these new codes near the existing ones.

For the reference, I used grep -r -E "'[a-z]'" in some directories to catch
these cases.

It is a bit off-topic for this thread but looking at the replication protocol
messages, a different pattern is used for naming the messages such as

WALData
Primary keepalive message
Standby status update
Hot standby feedback message
new archive
manifest
archive or manifest data
progress report

I would expect to see the same pattern (Pascal case) as the wire protocol.

AuthenticationSASLFinal
BackendKeyData
Bind

--
Euler Taveira
EDB https://www.enterprisedb.com/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Chao Li 2025-08-06 00:22:53 Re: Raw parse tree is not dumped to log
Previous Message Shinya Kato 2025-08-06 00:15:21 Re: Add backup_type to pg_stat_progress_basebackup