Hello.

Some messages recently introduced by commit 29d0a77fa6 seem to deviate
slightly from our standards.

+               if (*invalidated && SlotIsLogical(s) && IsBinaryUpgrade)
+               {
+                       ereport(ERROR,
+                                       
errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                       errmsg("replication slots must not be 
invalidated during the upgrade"),
+                                       errhint("\"max_slot_wal_keep_size\" 
must be set to -1 during the upgrade"));

The message for errhint is not a complete sentence. And errmsg is not
in telegraph style.  The first attached makes minimum changes.

However, if allowed, I'd like to propose an alternative set of
messages as follows:

+                                       errmsg("replication slot is invalidated 
during upgrade"),
+                                       errhint("Set \"max_slot_wal_keep_size\" 
to -1 to avoid invalidation."));

The second attached does this.

What do you think about those?

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 99823df3c7..15e85a23d4 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1433,8 +1433,8 @@ InvalidatePossiblyObsoleteSlot(ReplicationSlotInvalidationCause cause,
 		{
 			ereport(ERROR,
 					errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					errmsg("replication slots must not be invalidated during the upgrade"),
-					errhint("\"max_slot_wal_keep_size\" must be set to -1 during the upgrade"));
+					errmsg("replication slots must not be invalidated during upgrade"),
+					errhint("\"max_slot_wal_keep_size\" must be set to -1 during the upgrade."));
 		}
 
 		if (active_pid != 0)
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 99823df3c7..d561bd9be1 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1433,8 +1433,8 @@ InvalidatePossiblyObsoleteSlot(ReplicationSlotInvalidationCause cause,
 		{
 			ereport(ERROR,
 					errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					errmsg("replication slots must not be invalidated during the upgrade"),
-					errhint("\"max_slot_wal_keep_size\" must be set to -1 during the upgrade"));
+					errmsg("replication slot is invalidated during upgrade"),
+					errhint("Set \"max_slot_wal_keep_size\" to -1 to avoid invalidation."));
 		}
 
 		if (active_pid != 0)

Reply via email to