Re: Skipping logical replication transactions on subscriber side

From: Greg Nancarrow <gregn4422(at)gmail(dot)com>
To: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, "osumi(dot)takamichi(at)fujitsu(dot)com" <osumi(dot)takamichi(at)fujitsu(dot)com>, "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>, Alexey Lesovsky <lesovsky(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Skipping logical replication transactions on subscriber side
Date: 2021-08-16 03:03:24
Message-ID: CAJcOf-fotGnUyfUztmVK8JnTdv_7B1ixCLb5Kumopo3Lo4wAgA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Aug 16, 2021 at 6:24 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> Therefore, perhaps a message like "... in transaction 740 with commit
> timestamp 2021-08-10 14:44:38.058174+05:30" is better in terms of
> consistency with other messages?
>

Yes, I think that would be more consistent.

On another note, for the 0001 patch, the elog ERROR at the bottom of
the logicalrep_message_type() function seems to assume that the
unrecognized "action" is a printable character (with its use of %c)
and also that the character is meaningful to the user in some way.
But given that the compiler normally warns of an unhandled enum value
when switching on an enum, such an error would most likely be when
action is some int value that wouldn't be meaningful to the user (as
it wouldn't be one of the LogicalRepMsgType enum values).
I therefore think it would be better to use %d in that ERROR:

i.e.

+ elog(ERROR, "invalid logical replication message type %d", action);

Similar comments apply to the apply_dispatch() function (and I realise
it used %c before your patch).

Regards,
Greg Nancarrow
Fujitsu Australia

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2021-08-16 03:16:15 Re: Changes to recovery_min_apply_delay are ignored while waiting for delay
Previous Message Michael Paquier 2021-08-16 02:44:36 Re: badly calculated width of emoji in psql