Hello.
I noticed that the recent commit 0d48d393d46 introduced the following
three messages:
4793> errdetail("Retention is stopped as the apply process is not advancing its
xmin within the configured max_retention_duration of %u ms.",
4822> ? errdetail("Retention is re-enabled as the apply process is advancing
its xmin within the configured max_retention_duration of %u ms.",
4824> : errdetail("Retention is re-enabled as max_retention_duration is set to
unlimited."));
I think I saw other instances of this kind of as recently, and I
thought we had agreed to avoid this usage and prefer because instead,
but I lost track of where that discussion took place.
Anyway, unlike some past uses, these ones are apparently confusing,
and I'd like to propose changing the wording to because.
In addition, I felt that the tense in the second message is not
immediately clear. If it is reasonable and keeps the correct sense,
I'd like to propose changing "is (not) advancing its xmin within" to
"has (not) advanced its xmin into".
+ errdetail("Retention is stopped because the apply process has not advanced
its xmin into the configured max_retention_duration of %u ms.",
+ ? errdetail("Retention is re-enabled because the apply process has advanced
its xmin into the configured max_retention_duration of %u ms.",
+ : errdetail("Retention is re-enabled because max_retention_duration is set to
unlimited."));
I'm not sure this is worth fixing, but anyway the proposed patch is
attached.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
>From 871fb2e5c9c27dcc6bfef9d8acd30966b738261b Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <[email protected]>
Date: Tue, 16 Sep 2025 10:56:34 +0900
Subject: [PATCH] Reword recently introduced messages
Three error messages recently introduced by commit 0d48d393d46 use
somewhat confusing wording with as in the sense of because. In
addition, one of them employed a rather obscure tense. This patch
rewords both for clarity.
---
src/backend/replication/logical/worker.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 9b5885d57cf..24005568330 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -4790,7 +4790,7 @@ stop_conflict_info_retention(RetainDeadTuplesData *rdt_data)
ereport(LOG,
errmsg("logical replication worker for subscription \"%s\" has stopped retaining the information for detecting conflicts",
MySubscription->name),
- errdetail("Retention is stopped as the apply process is not advancing its xmin within the configured max_retention_duration of %u ms.",
+ errdetail("Retention is stopped because the apply process has not advanced its xmin into the configured max_retention_duration of %u ms.",
MySubscription->maxretention));
}
@@ -4819,9 +4819,9 @@ resume_conflict_info_retention(RetainDeadTuplesData *rdt_data)
errmsg("logical replication worker for subscription \"%s\" will resume retaining the information for detecting conflicts",
MySubscription->name),
MySubscription->maxretention
- ? errdetail("Retention is re-enabled as the apply process is advancing its xmin within the configured max_retention_duration of %u ms.",
+ ? errdetail("Retention is re-enabled because the apply process has advanced its xmin into the configured max_retention_duration of %u ms.",
MySubscription->maxretention)
- : errdetail("Retention is re-enabled as max_retention_duration is set to unlimited."));
+ : errdetail("Retention is re-enabled because max_retention_duration is set to unlimited."));
/*
* Restart the worker to let the launcher initialize
--
2.47.3
