public inbox for [email protected]  
help / color / mirror / Atom feed
From: Peter Smith <[email protected]>
To: Tom Lane <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: GUC names in messages
Date: Thu, 9 Nov 2023 12:55:44 +1100
Message-ID: <CAHut+Pv8VG7fvXzg5PNeQuUhJG17xwCWNpZSUUkN11ArV==Cdg@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAHut+Pv-kSN8SkxSdoHano_wPubqcg5789ejhCDZAcLFceBR-w@mail.gmail.com>
	<CAHut+PuDSj_knEzfXx+v9_-15XYSJAuFmpgubh8rN4nWjFMdUg@mail.gmail.com>
	<[email protected]>
	<[email protected]>

On Thu, Nov 2, 2023 at 1:25 AM Tom Lane <[email protected]> wrote:
>
...
> Our error message style guidelines say not to assemble messages out
> of separate parts, because it makes translation difficult.  Originally
> we applied that rule to GUC names mentioned in messages as well.
> Awhile ago the translation team decided that that made for too many
> duplicative translations, so they'd be willing to compromise on
> substituting GUC names.  That's only been changed in a haphazard
> fashion though, mostly in cases where there actually were duplicative
> messages that could be merged this way.  And there's never been any
> real clarity about whether to quote GUC names, though certainly we're
> more likely to quote anything injected with %s.  So that's why we have
> a mishmash right now.

Right. While looking at all the messages I observed a number of them
having almost the same (but not quite the same) wording:

For example,

errhint("Consider increasing the configuration parameter \"max_wal_size\".")));
errhint("You might need to increase %s.", "max_locks_per_transaction")));
errhint("You might need to increase %s.", "max_pred_locks_per_transaction")));
errmsg("could not find free replication state, increase
max_replication_slots")));
hint ? errhint("You might need to increase %s.", "max_slot_wal_keep_size") : 0);
errhint("You may need to increase max_worker_processes.")));
errhint("Consider increasing configuration parameter
\"max_worker_processes\".")));
errhint("Consider increasing the configuration parameter
\"max_worker_processes\".")));
errhint("You might need to increase %s.", "max_worker_processes")));
errhint("You may need to increase max_worker_processes.")));
errhint("You might need to increase %s.", "max_logical_replication_workers")));

~

The most common pattern there is "You might need to increase %s.".

Here is a patch to modify those other similar variations so they share
that common wording.

PSA.

======
Kind Regards,
Peter Smith.
Fujitsu Australia


Attachments:

  [application/octet-stream] v1-0001-Use-a-common-message-for-increasing-a-GUC.patch (4.0K, ../CAHut+Pv8VG7fvXzg5PNeQuUhJG17xwCWNpZSUUkN11ArV==Cdg@mail.gmail.com/2-v1-0001-Use-a-common-message-for-increasing-a-GUC.patch)
  download | inline diff:
From 5750183cb3236fede2e2d061544baa54783f1fd8 Mon Sep 17 00:00:00 2001
From: Peter Smith <[email protected]>
Date: Thu, 9 Nov 2023 12:34:15 +1100
Subject: [PATCH v1] Use a common message for increasing a GUC

---
 contrib/pg_prewarm/autoprewarm.c         | 4 ++--
 src/backend/postmaster/bgworker.c        | 2 +-
 src/backend/postmaster/checkpointer.c    | 2 +-
 src/backend/replication/logical/origin.c | 4 +++-
 src/test/modules/test_shm_mq/setup.c     | 2 +-
 5 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c
index d0efc9e..daf0bce 100644
--- a/contrib/pg_prewarm/autoprewarm.c
+++ b/contrib/pg_prewarm/autoprewarm.c
@@ -841,7 +841,7 @@ apw_start_leader_worker(void)
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_RESOURCES),
 				 errmsg("could not register background process"),
-				 errhint("You may need to increase max_worker_processes.")));
+				 errhint("You might need to increase %s.", "max_worker_processes")));
 
 	status = WaitForBackgroundWorkerStartup(handle, &pid);
 	if (status != BGWH_STARTED)
@@ -877,7 +877,7 @@ apw_start_database_worker(void)
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_RESOURCES),
 				 errmsg("registering dynamic bgworker autoprewarm failed"),
-				 errhint("Consider increasing configuration parameter \"max_worker_processes\".")));
+				 errhint("You might need to increase %s.", "max_worker_processes")));
 
 	/*
 	 * Ignore return value; if it fails, postmaster has died, but we have
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index 48a9924..7a8c8af 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -944,7 +944,7 @@ RegisterBackgroundWorker(BackgroundWorker *worker)
 								  "Up to %d background workers can be registered with the current settings.",
 								  max_worker_processes,
 								  max_worker_processes),
-				 errhint("Consider increasing the configuration parameter \"max_worker_processes\".")));
+				 errhint("You might need to increase %s.", "max_worker_processes")));
 		return;
 	}
 
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index a3c1aba..3dab5f8 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -423,7 +423,7 @@ CheckpointerMain(void)
 									   "checkpoints are occurring too frequently (%d seconds apart)",
 									   elapsed_secs,
 									   elapsed_secs),
-						 errhint("Consider increasing the configuration parameter \"max_wal_size\".")));
+						 errhint("You might need to increase %s.", "max_wal_size")));
 
 			/*
 			 * Initialize checkpointer-private variables used during
diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c
index b0255ff..fca8924 100644
--- a/src/backend/replication/logical/origin.c
+++ b/src/backend/replication/logical/origin.c
@@ -795,7 +795,9 @@ StartupReplicationOrigin(void)
 		if (last_state == max_replication_slots)
 			ereport(PANIC,
 					(errcode(ERRCODE_CONFIGURATION_LIMIT_EXCEEDED),
-					 errmsg("could not find free replication state, increase max_replication_slots")));
+					 errmsg("could not find free replication state"),
+					 errhint("You might need to increase %s.",
+							 "max_replication_slots")));
 
 		/* copy data to shared memory */
 		replication_states[last_state].roident = disk_state.roident;
diff --git a/src/test/modules/test_shm_mq/setup.c b/src/test/modules/test_shm_mq/setup.c
index abc7935..d00648e 100644
--- a/src/test/modules/test_shm_mq/setup.c
+++ b/src/test/modules/test_shm_mq/setup.c
@@ -233,7 +233,7 @@ setup_background_workers(int nworkers, dsm_segment *seg)
 			ereport(ERROR,
 					(errcode(ERRCODE_INSUFFICIENT_RESOURCES),
 					 errmsg("could not register background process"),
-					 errhint("You may need to increase max_worker_processes.")));
+					 errhint("You might need to increase %s.", "max_worker_processes")));
 		++wstate->nworkers;
 	}
 
-- 
1.8.3.1



view thread (2+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: GUC names in messages
  In-Reply-To: <CAHut+Pv8VG7fvXzg5PNeQuUhJG17xwCWNpZSUUkN11ArV==Cdg@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox