On Mon, Aug 3, 2026 at 8:35 AM Imran Zaheer <[email protected]> wrote:
> Hi Jakub, thanks a lot for the quick review, and sorry for the late reply. Hi Imran, > > a. with original post (v1 patch) you have reported ~42% gain for > > simple-update, but now it's just down to just 14% or 31%, any idea why? > > > > I am actually not sure about that. I should have done some long-running > pgbenches. I think using shorter replays was not a good idea for judging the > performance. This time I did some longer benches [1] of 30 mins, and it turned > out the pipeline gains are decreased relatively to the last run. The > simple-update > s-buff cases decreased from ~30% to ~20%. That doesn't sound good. Looks > like the longer replays have something that is killing pipeline gains. Well maybe you are hitting some thermal issues? Fundamentally I do not see any thing that would impact longer running processes other than that. Anyway be sure to use something like below receipe to stabilize the benchmark runs: # if on NUMA, prefeably as root numactl --cpunodebind=0 --membind=0 bash cpupower frequency-set -g performance cpupower idle-set -D0 # careful, be sure to revert/reboot, it may cook stuff! echo 0 > /sys/devices/system/cpu/cpufreq/boost # Intel, AMD had something else # probably optional due to above idle-set, but still: echo 2000000 | sudo tee /sys/devices/system/cpu/cpu*/cpufreq/scaling_min_freq # not sure on 1st, but THP is known maybe cause to coalesce code/ELF(!) pages # which influence code efficency (iTLB %) echo never > /sys/kernel/mm/transparent_hugepage/defrag echo never > /sys/kernel/mm/transparent_hugepage/enabled Do several (3-5?) runs, warm-up first (and ignore that result) for like 30-60s, perform drop_caches across runs. It should give you consistency (assuming nothing else would be running in parallel). Perhaps also try to add another scenario with just plain INSERTs on table without any indexes, it is much more predictible and avoid any UPDATEs in the WAL stream while microbenchmarking this as heap_page_update() is kind of complex and may do PageRepairFragmentation() and memcpy() in the startup... Anyway you could also check the every-10s startup CPU stats (log_startup_progress_interval to 1s?) somehow combined with LSN calculations (diff of LSN [ratio LSN processed vs time], given that generator would be using the same WAL stream You could notice the moment of the slow down... just an idea. > But the overall performance gains by enabling the pipeline were more or less > the > same. I have added some detailed bench scenarios in the pdf report by > running a single recovery with 4 diff scenarios (s-b & bgwriter combinations). > Benches showed that making bgwriter aggressive did offload the > writes and decreased some recovery time but related performance gain > by enabling the pipeline was almost the same in some cases (c-t-5 & c-t-7) and > was prominent in some (see tests c-t-1 & c-t-3). > > I also did the archive recovery this time. Before that, we were > mimicking a crash > recovery by copying all the archived wal to pg_wal beforehand and then > starting > the cluster. But we skip this in archive recovery and postgres will itself > copy each wal file via restore command (cp in this case). Due to the > archive recovery, there was a change in the workflow. Now XlogPageRead() > triggers WaitForWALToBecomeAvailable() more often. You can see this > shift in the flamegraphs. Also, enabling the pipeline will shift this > wait from the > startup to the producer worker. Benches will show different pipeline > gains for an archive & a crash recovery even with the same workloads. > For example, > see the 'perf' column in the pdf report for a-t-7, a-t-8 & c-t-7, c-t-8. > My guess for the perf decrease is that even if the pipeline offloads decoder > from the startup proc and makes the startop proc fast, but producer > may now have > to wait for wal to arrive (WaitForWALToBecomeAvailable). This can create a > slow > producer and fast consumer scenario. > [..] > At last, after doing the benches, it looks like the pipeline isn't > doing well in the longer runs > compared to the last run. I will try to do some more R&D on that. > > Thanks for the quick review, and will be looking forward to hearing > more from you. > > [1]: > https://drive.google.com/file/d/1cSzADxXaDWBCXBoJMr58kUnhFHNBx3pT/view?usp=sharing > > Test script: https://github.com/imranzaheer612/pg-recovery-testing The very good news is that I could have standby running without crashes and even throwing pg_regress workload onto primary it did not crash standby with WAL pipeline running (so lots of progress since last time! :)). I've took a another rather quick look (for the record, not reviewing corectness just trying to see performance of just 1.2x, but I've failed), so I've used old Thomas script [1] which we used back in the day of previous WAL optimizations (cached lseek optimizatiosn, and prefetching work and other similiar findings) to rule out anything coming from Your's testing scripts. Slight nano changes there: s_b=1GB, and those aggressive bgwritter settings, crash-recovery scenario produced from classic pgbech OLTP ~4GB of pgdata.crash with 3.2GB of WAL, mostly without FPIs and with like ~50% of Heap/HOT_UPDATE in the WAL stream just to compare to Your's numbers). I do think Your's better results with larger s_b are just attributed to having less FPIs (less hit from memcpy() to/from mq) Avg od 3 runs: scenario walltime CPUtime wal_pipeline=off (baseline) 19.6s 18.9 s wal_pipeline=on 17.7s 27.9 s (2 processes) so rougly I've got just 1.10x with this which is not good when in theory adding another CPU. Overwall, the typical CPU picture was something like: ~30% bgwriter, 99% startup/recovery, 55-65% wal producer. Notes/findings as they were discovered: 0. the new process shouldn't in process list as 'wal pipeline producer recovering <WALSEG>'. Instead it should say something more 'wal pipeline decoding <WALSEG>' (or reading, because it's not recovering). 1. strace said it's bascially flood of kill(postmaster_PID, SIGURG) from that 'wal pipline producer' rarely interrupted by meanigful work. It's apparently coming from WalPipeline_SendRecord()->shm_mq_sendv()->kill(), it's apparently due to the shm_sendv(.... , notify=true). I've set it to false and it did help to get ~ -10% CPU on the 2nd process, but didn't gain much on the whole result. I'm afraid we still need to issue that kill() from time to time, but not sure how often (e.g. sending kill maximum every 100ms(?) should be good enough, with just notify=false we it is serious bug as we won't notify startup/recovery, so it appears as hanging until the shm mq is non-empty) 2. profiling said it's constant mutex hit, so I've took a look and found out that You are using plenty of spinlocks for very basic stuff such as just chaning single bools and/or updating stats, e.g: SpinLockAcquire(&WalPipelineShm->mutex); WalPipelineShm->producerWaiting = true; SpinLockRelease(&WalPipelineShm->mutex); so with attached I've replaced that with atomics all over the place and that thing started to fly for me: Avg od 3 runs: scenario walltime wal_pipeline=off (baseline) 19.5s wal_pipeline=on 18.0s wal_pipeline=on+atomics 14.4s 19.5/14.4=1.35x (with basic pgbench OLTP) and I think You are going to get better numbers especially for the other kinds of traffic. I'm was not really sure which of those were really needed to be adjusted like that, so this is with "big-bang" and every variable is flagged to have dedicated cache lines (64b is not much for such gains, hope it can stay like that). 3. Some part of me do not like the pg_usleep(50us) in the WalPipeline_WaitForConsumeCatchup(), but I do not have better idea how to semi-actively wait there... or maybe it is OK (?) 4. Having patch applied, I'm still getting like 99% : 55% (2:1) CPU ratio split of work between startup and wal pipeline, so possibly something could be shifted more to that pipeline. Maybe please re-test perf with attached as You were getting much better results. Earlier (in 1st thread msg) You have written that You considered pinning buffers from 2nd process (and pass those already pinned), it's an open question for me too, should it be done, but maybe some committers could chime in (it's basically tradeoff: complexity-for-performance, right?) 5. We should avoid having "_" in the function names I think, so nothing seems to be named like that (You mix uppercase with "_" within single function name) 6. In v06-0005 you have introduced timeval_substract() and _add(), but those possibly should go into pg_rusage.c for code re-usability in separate commit before that 0005. -J. [1] - https://github.com/macdice/redo-bench
v06_18082026-0001-Use-atomics-for-speed-instead-of-spinlo.patch.nocfbot
Description: Binary data
