On Fri, Aug 28, 2026 at 10:44 PM Gustavo William <[email protected]> wrote: > > Hi, > > I'm implementing an improvement for incremental backups (.txt patch attached) > which I think is relevant to this thread. > > Context: I noticed pg_basebackup incremental backups can sometimes be very > slow, sometimes even slower than a full backup. The more scattered the > changed blocks, the worse it gets. Looking at disk metrics and the code I > found one of the reasons is that delta blocks are requested serially, each > changed block read strictly one at a time in a loop. > > A simple fix, while still keeping the single-threaded design, was to issue > posix_fadvise() calls for upcoming blocks before the code gets to them, in > the hope that the kernel brings them into cache before they're actually read. > > A few results so far, on a ~51GB cluster: > ~10 min of pgbench running, then taking an incremental backup of 28GB took > 2min 51s (~167.7 MiB/s). With the patched code: 2min 18s (~207.8 MiB/s), ~24% > higher. > With more scattered changes (via TABLESAMPLE SYSTEM(20), touching ~20% of a > table uniformly at random): 31GB incremental in 4min 22s (~121.2 MiB/s). With > the patched code: 2min 32s (~208.8 MiB/s), ~72% higher. > > Short overview of the patch: > 1. Before iterating over the delta blocks, fadvise the first N blocks. > 2. After each iteration (each block read), fadvise the block at > positioncurrent_block + N. > 3. Repeat this every iteration until the end. > > This way, we're always at least N blocks ahead of the current one. For now, N > = maintenance_io_concurrency. > > The logic is similar to patch 0005 from Jakub, but for the random read > pattern instead. > > I intend to keep working on this in a couple of days (add tests, collect > metrics), but wanted to share where it stands now in case anyone has thoughts > in the meantime.
Hi Gustavo, I would suggest we merge Your's and mine into one bigger combined 0005 later unless someone opposes (we would be both coauthors). The reason is that aparently I've forgotten to support incremental mode (which means that I ended up issuing posix_fadvise SEQUENTIAL even in incremental mode, which is not necessary) and Your's introduces concept of advising for only needed blocks in the incremental path, but first, my review of Your's one: 1. there's bug that when m_io_c = 0 you still issue posix_fadvise()s from 2nd callsite (no way to turn it off) and also with e.g. m_io_c = 1 we seem to still issue an additional one. 2. I'm not sure f that merging of adjacent blocks shouldn't be also capped up to a io_combine_limit GUC (by default 16, so that's like 128kB per advise). I mean, OSes should accept it, but based on my research they could silently clamp it to sysfs per-device "read_ahead_kb" or 2MB (?), so if we would merge here like way more and request e.g. 5MB in a single fadvise, it would be simply ignored (??) and that would be a no-op, and after some time fadvise performance effects would be lost. So perhaps it's better to limit ourselves to that GUC (?) That would mean we would somehow need to adjust the algorithm to fulfill the full window of prefetches from the 2nd callsite too. 3. I would rename pf_index to just prefetch_idx, so it would easier to read. -J.
