Re: log XLogPrefetch stats at end of recovery

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
To: Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>
Cc: Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com>, SATYANARAYANA NARLAPURAM <satyanarlapuram(at)gmail(dot)com>, Lakshmi N <lakshmin(dot)jhs(at)gmail(dot)com>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: log XLogPrefetch stats at end of recovery
Date: 2026-11-04 18:46:52
Message-ID: CALj2ACXkE9saBJDhHUbtpJ6mKUQ7t-LvcgfxUvkt6KnmzhVX2Q@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2026-11-04 18:47:22 Re: Unlogged bloom indexes break after crash recovery
Previous Message Andres Freund 2026-11-04 18:45:35 Re: PG19 regression in ecd9288624a: JSON_ARRAY() indexes that go stale