On 2025-03-27 11:06, torikoshia wrote:
Hi,

I had another off-list discussion with Fujii-san, and according to the
following manual[1], it seems that a transaction with an overflowed
subtransaction is already considered inconsistent:

  Reaching a consistent state can also be delayed in the presence of
both of these conditions:

  - A write transaction has more than 64 subtransactions
  - Very long-lived write transactions

IIUC, the manual suggests that both conditions must be met -- recovery
reaching at least minRecoveryPoint and no overflowed subtransactions
—- for the standby to be considered consistent.

OTOH, the following log message is emitted even when subtransactions
have overflowed, which appears to contradict the definition of
consistency mentioned above:

  LOG:  consistent recovery state reached

This log message is triggered when recovery progresses beyond
minRecoveryPoint(according to CheckRecoveryConsistency()).
However, since this state does not satisfy 'consistency' defined in
the manual, I think it would be more accurate to log that it has
merely reached the "minimum recovery point".
Furthermore, it may be better to emit the above log message only when
recovery has progressed beyond minRecoveryPoint and there are no
overflowed subtransactions.

Attached patch does this.

Additionally, renaming variables such as reachedConsistency in
CheckRecoveryConsistency might also be appropriate.
However, in the attached patch, I have left them unchanged for now.


On 2025-03-25 00:55, Fujii Masao wrote:
-                  case CAC_NOTCONSISTENT:
+                  case CAC_NOTCONSISTENT_OR_OVERFLOWED:

This new name seems a bit too long. I'm OK to leave the name as it is.
Or, something like CAC_NOTHOTSTANDBY seems simpler and better to me.

Beyond just the length issue, given the understanding outlined above,
I now think CAC_NOTCONSISTENT does not actually need to be changed.


In high-availability.sgml, the "Administrator's Overview" section already
describes the conditions for accepting hot standby connections.
This section should also be updated accordingly.

Agreed.
I have updated this section to mention that the resolution is to close
the problematic transaction.
OTOH the changes made in v2 patch seem unnecessary, since the concept
of 'consistent' is already explained in the "Administrator's
Overview."


- errdetail("Consistent recovery state has not been yet reached.")));
+                                                        errdetail("Consistent 
recovery state has not been yet
reached, or snappshot is pending because subtransaction is
overflowed."),

Given the above understanding, "or" is not appropriate in this
context, so I left this message unchanged.
Instead, I have added an errhint. The phrasing in the hint message
aligns with the manual, allowing users to search for this hint and
find the newly added resolution instructions.

On second thought, it may not be appropriate to show this output to clients attempting to connect. This message should be notified not to clients but to administrators.

From this point of view, it'd be better to output a message indicating the status inside ProcArrayApplyRecoveryInfo(). However, a straightforward implementation would result in the same message being logged every time an XLOG_RUNNING_XACTS WAL is received, making it noisy.

Instead of directly outputting a log indicating that a hot standby connection cannot be established due to subtransaction overflow, the attached patch updates the manual so that administrators can determine whether a subtransaction overflow has occurred based on the modified log output.


What do you think?


--
Regards,

--
Atsushi Torikoshi
Seconded from NTT DATA GROUP CORPORATION to SRA OSS K.K.
From 908db07687d9d4473ab86d93356b7d605329c0be Mon Sep 17 00:00:00 2001
From: Atsushi Torikoshi <torikos...@oss.nttdata.com>
Date: Thu, 27 Mar 2025 23:50:08 +0900
Subject: [PATCH v4] Modify and log to align with the doc

There is an inconsistency between the documentation[1] and the log
messages regarding the definition of a 'consistent' recovery state.
The documentation states that a consistent state requires both that
recovery has progressed beyond minRecoveryPoint and that there are
no overflowed subtransactions, whereas the source code previously
considered only minRecoveryPoint.

This patch modifies and adds log messages to align with the
documentation. Specifically, messages are now logged both when
recovery progresses beyond minRecoveryPoint and when the system
reaches a consistent state in the updated definition. As a result,
the "consistent recovery state reached" message is now output
later than before.

As a side effect, previously, if hot standby remained inaccessible
due to overflowed subtransactions, it was difficult to determine
the cause. With this change in log messages and the corresponding
update to the documentation, the reason for the delay can now be
identified more clearly.

[1] https://www.postgresql.org/docs/devel/hot-standby.html
---
 doc/src/sgml/high-availability.sgml       | 8 ++++++++
 src/backend/access/transam/xlogrecovery.c | 5 ++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml
index acf3ac0601..c83c5403ed 100644
--- a/doc/src/sgml/high-availability.sgml
+++ b/doc/src/sgml/high-availability.sgml
@@ -1967,6 +1967,10 @@ LOG:  entering standby mode
 
 ... then some time later ...
 
+LOG:  minimum recovery point reached
+
+... It may take some time ...
+
 LOG:  consistent recovery state reached
 LOG:  database system is ready to accept read-only connections
 </programlisting>
@@ -1991,6 +1995,10 @@ LOG:  database system is ready to accept read-only connections
        </listitem>
       </itemizedlist>
 
+    In this case, only "minimum recovery point reached" is logged, and
+    "consistent recovery state reached" is not logged. This issue can be
+    resolved by closing the transaction.
+    This case can be resolved by closing the transaction.
     If you are running file-based log shipping ("warm standby"), you might need
     to wait until the next WAL file arrives, which could be as long as the
     <varname>archive_timeout</varname> setting on the primary.
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index 2c19013c98..8152f90c99 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -2249,7 +2249,7 @@ CheckRecoveryConsistency(void)
 
 		reachedConsistency = true;
 		ereport(LOG,
-				(errmsg("consistent recovery state reached at %X/%X",
+				(errmsg("minimum recovery point reached at %X/%X",
 						LSN_FORMAT_ARGS(lastReplayedEndRecPtr))));
 	}
 
@@ -2268,6 +2268,9 @@ CheckRecoveryConsistency(void)
 		SpinLockRelease(&XLogRecoveryCtl->info_lck);
 
 		LocalHotStandbyActive = true;
+		ereport(LOG,
+				(errmsg("consistent recovery state reached at %X/%X",
+						LSN_FORMAT_ARGS(lastReplayedEndRecPtr))));
 
 		SendPostmasterSignal(PMSIGNAL_BEGIN_HOT_STANDBY);
 	}

base-commit: 0f3604a518f8b3fd35ffc344d85c71693ded0dde
-- 
2.48.1

Reply via email to