From: | Peter Smith <smithpb2250(at)gmail(dot)com> |
---|---|
To: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
Cc: | Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Greg Stark <stark(at)mit(dot)edu>, Euler Taveira <euler(at)eulerto(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: add log messages when replication slots become active and inactive (was Re: Is it worth adding ReplicationSlot active_pid to ReplicationSlotPersistentData?) |
Date: | 2023-11-15 22:18:29 |
Message-ID: | CAHut+PvuKzniJ3U-V+XxGb8m0Cj2gQgtkjW3QPBFYk5GgCyLrg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Some minor comments for v17-0001.
======
1.
+ ereport(log_replication_commands ? LOG : DEBUG1,
+ SlotIsLogical(s)
+ /* translator: %s is name of the replication slot */
+ ? errmsg("acquired logical replication slot \"%s\"",
+ NameStr(s->data.name))
+ : errmsg("acquired physical replication slot \"%s\"",
+ NameStr(s->data.name)));
1a.
FWIW, if the ternary was inside the errmsg, there would be less code
duplication.
~
1b.
I searched HEAD code and did not find any "translator:" comments for
just ordinary slot name substitutions like these; AFAICT that comment
is not necessary anymore.
~
SUGGESTION (#1a and #1b)
ereport(log_replication_commands ? LOG : DEBUG1,
errmsg(SlotIsLogical(s)
? "acquired logical replication slot \"%s\""
: "acquired physical replication slot \"%s\"",
NameStr(s->data.name)));
~~~
2.
Ditto for the other ereport.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
From | Date | Subject | |
---|---|---|---|
Next Message | Thomas Munro | 2023-11-15 22:40:07 | Re: On non-Windows, hard depend on uselocale(3) |
Previous Message | Nathan Bossart | 2023-11-15 21:48:53 | Re: Popcount optimization using AVX512 |