Re: log XLogPrefetch stats at end of recovery

Hi Bharath,

On Thu, Nov 5, 2026 at 12:17 AM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Wed, Oct 14, 2026 at 9:05 AM Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> wrote:
> >
> > I have created a CF entry for this in the name of Lakshmi N (the
> > author of this patch). Also, I find that the patch has been idle for
> > months and no longer applied, so I have rebased it (v4 attached) with
> > some cosmetic changes.
>
> Thanks. I reviewed the v4 patch.
>
> I think reporting the prefetch stats into the server logs at the end
> of recovery is useful for post hoc analysis, especially for long
> recovery times, e.g., after a standby is promoted, after a long crash
> recovery, or when recovering from a backup/snapshot. And, one can know
> the prefetch progress using the stats function anyway.
>
> Few comments:
>
> 1/ Using PRIu64 here looks good to me. Commits 15a79c731, a0ed19e0a. I
> see some usages of PRId64 and PRIu64 for int64 and uint64 (WalUsage
> for example).
>
> 2/ I think reporting it after reading the last valid or applied record
> when finishing the WAL recovery would cover the last record's stats as
> well, but the difference in the stats might be small, so reporting it
> here after the redo loop is fine with me.
>
> 3/
> +void
> +XLogPrefetchLogStats(void)
> +{
> + if (recovery_prefetch == RECOVERY_PREFETCH_OFF)
> + return;
>
> Use RecoveryPrefetchEnabled() instead. Prefetch is enabled only on the
> supported platforms and io concurrency also needs to be set.
>
> 4/ I think, in case of long recoveries, not reporting the accumulated
> stats, if any, before prefetch was turned off seems a bit incomplete
> and inconsistent. How about we just log the stats as-is (it's fine for
> them to be all zeros), and also optionally log the value of the GUC as
> of that moment? This log message is not going to flood the server
> logs, as it happens only at the end of the recovery anyway.
>
> 5/
> +/*
> + * Log a summary of the prefetch counters at the end of recovery.
> + */
> +void
> +XLogPrefetchLogStats(void)
>
> I don't think there's anything that makes this function specific to
> being called "at the end of recovery," other than the "redo" in the
> log message. One can call it at any point in time and get the
> accumulated stats as of that moment. This makes me think whether the
> function needs to log the redo-specific message at all, or instead do
> something like pg_rusage_show() does and leave the redo message to the
> caller. I don't like exposing the shared stats pointer out of
> xlogprefetcher.c. Or just rename the function to
> XLogPrefetchLogStatsOnRecoveryEnd(). I prefer to rename it.
>

Thanks for reviewing the patch.

I have addressed comments 4 and 5 in the attached v5 patch. Comments 1
and 2 do not appear to require changes and comment 3 conflicts with
comment 4, and I chose to implement 4 over 3.

To summarize, v5 adds these:

1) Reports accumulated prefetch statistics at the end of recovery,
regardless of whether prefetching is enabled at that point, and
includes the current recovery_prefetch setting.
2) Adds regression tests covering zero counters and statistics
retained after disabling prefetching.
3) Renames the function to XLogPrefetchLogStatsOnRecoveryEnd() to make
its intended use explicit.

Please take a look and let me know for any other comments.

--
With Regards,
Ashutosh Sharma.