On Wed, Jul 23, 2025 at 05:04:37AM +0000, Hayato Kuroda (Fujitsu) wrote: > While working on [1] I found the point. When recovery_target_lsn is specified > and > recovery_target_inclusive is false, recoveryStopsAfter() cannot return true. > > /* Check if the target LSN has been reached */ > if (recoveryTarget == RECOVERY_TARGET_LSN && > recoveryTargetInclusive && > record->ReadRecPtr >= recoveryTargetLSN) > > In this case the recovery can stop when next record is read. This normally > works > well but if the next record has not been generated yet, startup process will > wait > till new one will come then exit from the apply loop.
+static inline bool +TargetLSNIsReached(XLogReaderState *record, bool is_before) +{ + XLogRecPtr targetLsn; + + Assert(recoveryTarget == RECOVERY_TARGET_LSN); + + if (is_before) + { + if (recoveryTargetInclusive) + return false; + + targetLsn = record->ReadRecPtr; + } + else + { + targetLsn = recoveryTargetInclusive ? + record->ReadRecPtr : record->EndRecPtr; + } + + return targetLsn >= recoveryTargetLSN; +} So, what you are doing here is changing the behavior of the check in recoveryStopsAfter(), with the addition of one exit path based on EndRecPtr if recovery_target_inclusive is false, to leave a bit earlier in case if we don't have a follow-up record. Did you notice a speedup once you did that in your logirep workflow? I am afraid that this change would be a new mode, skipping a couple of code paths if one decides to trigger the exit. And I highly suspect that you could break some cases that worked until now here, skipping one recoveryPausesHere() and one ProcessStartupProcInterrupts() to say the least. > I feel the process can exit bit earliyer, by comparing with the end point of > the > applied record and recovery_target_lsn. > Attached patch roughly implemented the idea. Could you prove your point with a test or something that justifies a new definition? > I read the old discussions, but I cannot find the reason of current style. 35250b6ad7a8 was quite some time ago, as of this thread, with my fingerprints all over it: https://www.postgresql.org/message-id/flat/CAB7nPqRKKyZQhcB39mty9C_gfB0ODRUQZEWB4GPP8ARpdAB%3DZA%40mail.gmail.com If my memory serves me right, this comes down to the consistency with how stop XID targets are handled before and after records are read, except that this one applies to ReadRecPtr. -- Michael
signature.asc
Description: PGP signature