Re: Log prefix missing for subscriber log messages received from publisher

On Wed, Jul 16, 2025 at 2:24 AM vignesh C <vignesh21@gmail.com> wrote:
>
> On Mon, 14 Jul 2025 at 21:36, Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
> >
> >
> >
> > On 2025/04/15 13:37, vignesh C wrote:
> > > Hi,
> > >
> > > Currently, when a warning is emitted by the publisher, the
> > > corresponding log message does not include the log prefix. This makes
> > > it harder to correlate such messages with other log entries. For
> > > example, in a simulated error scenario where directory removal fails,
> > > the notice message lacks the standard log prefix, as shown below:
> > > 2025-03-18 16:44:36.071 IST [196901] LOG:  logical replication table
> > > synchronization worker for subscription "sub1", table "t1" has
> > > finished
> > > WARNING:  could not remove directory
> > > "pg_replslot/pg_16398_sync_16387_7483106341004194035.tmp"
> > >
> > > In this case, the WARNING line does not include the usual timestamp
> > > information, making it harder to trace.
> > >
> > > To address this, we can have a custom notice processor for WAL
> > > receiver connections—similar to what's done in the attached patch.
> >
> > I like this idea.
> >
> > If this issue also exists other features like dblink or postgres_fdw
> > connecting to remote PostgreSQL servers, it might be worth applying
> > a similar improvement there as well.
>
> Included it for dblink and fdw

Thanks! It's better to submit each change for dblink and postgres_fdw
as separate patches, and first focus on the patch that sets the notice
processor for replication connections.


> > +notice_processor(void *arg, const char *message)
> > +{
> > +       elog(LOG, "%s", message);
> >
> > Should ereport() be used here instead?
>
> Yes, that is better. Modified it to ereport
>
> > Also, would it be better to add some context before %s, for example,
> > something like "received message from the primary:"? Without that,
> > users might mistakenly think the message originated from the local server.
>
> Yes, that makes sense. Updated accordingly.
>
> For dblink and postgres_fdw, create the dblink and postgres_fdw setup
> and create the below object to raise a notice:
> CREATE OR REPLACE FUNCTION emit_notice(msg text) RETURNS int AS $$
> BEGIN
>     RAISE NOTICE '%', msg;
>     RETURN 1;
> END;
> $$ LANGUAGE plpgsql;
>
> CREATE VIEW my_notice_view AS SELECT 1 AS value;
>
> CREATE OR REPLACE RULE "_RETURN" AS
>     ON SELECT TO my_notice_view
>     DO INSTEAD
>     SELECT 1 AS value
>     FROM (SELECT emit_notice('NOTICE from _RETURN rule') AS dummy) s;
>
> -- Scenario to see the notice raised by remote server using dblink
> SELECT * FROM dblink('dbname=remotedb user=remoteuser',  'SELECT *
> FROM my_notice_view') AS t(value int);
>
> --  Scenario to see the notice raised by remote server using postgres_fdw
> IMPORT FOREIGN SCHEMA public LIMIT TO (my_notice_view) FROM SERVER
> foreign_server INTO public;
>
> The attached v2 version patch has the changes for the same.

Thanks for updating the patch!

+ /* Trim trailing newline for cleaner logs */
+ size_t len = strlen(message);
+
+ if (len > 0 && message[len - 1] == '\n')
+ ereport(LOG,
+ errmsg("Received message from publisher: %.*s",
+ (int) (len - 1), message));

Do we really need to trim the trailing newline? Are there actual
WARNING/NOTICE/INFO messages that include an unnecessary newline?

"Received" should be lowercase, i.e., "received" since the primary log message
should start with a lowercase letter, according to the Error Message
Style Guide [1].

The phrase "from publisher" could be misleading, since the sender might be
a primary in physical replication, not necessarily a logical publisher.
Maybe something like "received message via replication" would be clearer?
I'd like to hear others' thoughts on a better wording.

Regards,

[1] https://www.postgresql.org/docs/devel/error-style-guide.html

--
Fujii Masao