Re: Introduce XID age and inactive timeout based replication slot invalidation - Mailing list pgsql-hackers

Hi Nisha.

Some review comments for patch v67-0001.

======
src/backend/replication/slot.c

ReportSlotInvalidation:

Please see my previous post [1] where I gave some reasons why I think
the _() macro should be used only for the *common* part of the hint
messages. If you agree, then the following code should be changed.

1.
+ /* translator: %s is a GUC variable name */
+ appendStringInfo(&err_hint, "You might need to increase \"%s\".",
+ "max_slot_wal_keep_size");
  break;

Change to:
_("You might need to increase \"%s\".")

~

2.
+ /* translator: %s is a GUC variable name */
+ appendStringInfo(&err_hint, "You might need to increase \"%s\".",
+ "idle_replication_slot_timeout");
+ break;

Change to:
_("You might need to increase \"%s\".")

~

3.
- hint ? errhint("You might need to increase \"%s\".",
"max_slot_wal_keep_size") : 0);
+ err_hint.len ? errhint("%s", _(err_hint.data)) : 0);

Change to:
err_hint.len ? errhint("%s", err_hint.data) : 0);

======
[1] https://www.postgresql.org/message-id/CAHut%2BPuKCv-S%2BPJ2iybZKiqu0GJ1fSuzy2CcvyRViLou98QpVA%40mail.gmail.com

Kind Regards,
Peter Smith.
Fujitsu Australia