Hi Bharath, On Thu, Nov 5, 2026 at 12:17 AM Bharath Rupireddy <[email protected]> wrote: > > Hi, > > On Wed, Oct 14, 2026 at 9:05 AM Ashutosh Sharma <[email protected]> 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.
v5-0001-xlogprefetcher-Log-prefetch-statistics-at-end-of-rec.patch
Description: Binary data
