On Mon, Apr 4, 2022 at 3:12 PM Julien Rouhaud <rjuju...@gmail.com> wrote: > [review]
Thanks! I took almost all of your suggestions about renaming things, comments, docs and moving a magic number into a macro. Minor changes: 1. Rebased over the shmem stats changes and others that have just landed today (woo!). The way my simple SharedStats object works and is reset looks a little primitive next to the shiny new stats infrastructure, but I can always adjust that in a follow-up patch if required. 2. It was a bit annoying that the pg_stat_recovery_prefetch view would sometimes show stale numbers when waiting for WAL to be streamed, since that happens at arbitrary points X bytes apart in the WAL. Now it also happens before sleeping/waiting and when recovery ends. 3. Last year, commit a55a9847 synchronised config.sgml with guc.c's categories. A couple of hunks in there that modified the previous version of this work before it all got reverted. So I've re-added the WAL_RECOVERY GUC category, to match the new section in config.sgml. About test coverage, the most interesting lines of xlogprefetcher.c that stand out as unreached in a gcov report are in the special handling for the new CREATE DATABASE in file-copy mode -- but that's probably something to raise in the thread that introduced that new functionality without a test. I've tested that code locally; if you define XLOGPREFETCHER_DEBUG_LEVEL you'll see that it won't touch anything in the new database until recovery has replayed the file-copy. As for current CI-vs-buildfarm blind spots that recently bit me and others, I also tested -m32 and -fsanitize=undefined,unaligned builds. I reran one of the quick pgbench/crash/drop-caches/recover tests I had lying around and saw a 17s -> 6s speedup with FPW off (you need much longer tests to see speedup with them on, so this is a good way for quick sanity checks -- see Tomas V's results for long runs with FPWs and curved effects). With that... I've finally pushed the 0002 patch and will be watching the build farm.
