From: Tom Lane <tgl@sss.pgh.pa.us>
To: Dimitrios Apostolou <jimis@gmx.net>
cc: Chao Li <li.evan.chao@gmail.com>,
        Nathan Bossart <nathandbossart@gmail.com>,
        Thomas Munro <thomas.munro@gmail.com>,
        pgsql-hackers@lists.postgresql.org
Subject: Re: [PATCH v4] parallel pg_restore: avoid disk seeks when jumping
 short distance forward
In-reply-to: <9opr64ps-625r-667n-q19o-op35rs414n59@tzk.arg>
References: <2edb7a57-b225-3b23-a680-62ba90658fec@gmx.net>
 <a7344d8f-927c-1f97-6cc0-ac630deb98c4@gmx.net>
 <c0e531f9-accc-3a93-e9e6-2cec1471f9e9@gmx.net>
 <1282067.1744651649@sss.pgh.pa.us>
 <c6bb93c1-4042-743c-096c-27c5e7c73347@gmx.net>
 <f7a23708-c336-c189-f155-4b3197bf6ea0@gmx.net> <aEioDuePEBRnfJYk@nathan>
 <5ed51587-5a24-4d79-e36e-927620d0e3a2@gmx.net> <aEygf8AchBk04gOA@nathan>
 <f99e9774-da49-60f0-20e0-b97d9f12a04e@gmx.net>
 <72d2c81b-a0fe-3d99-9dd4-0d771e6e673e@gmx.net>
 <1po8os49-r63o-2923-p37n-12698o1qn7p0@tzk.arg>
 <3515357.1760128017@sss.pgh.pa.us> <3698234.1760149336@sss.pgh.pa.us>
 <3686.1760232320@sss.pgh.pa.us>
 <CBF70398-279C-4BA7-9FA1-2879A49F4EC9@gmail.com>
 <717335.1760389672@sss.pgh.pa.us> <1306492.1760556110@sss.pgh.pa.us>
 <1518975.1760634070@sss.pgh.pa.us>
 <9opr64ps-625r-667n-q19o-op35rs414n59@tzk.arg>
Comments: In-reply-to Dimitrios Apostolou <jimis@gmx.net>
	message dated "Mon, 20 Oct 2025 20:40:27 +0200"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <375561.1760995276.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Mon, 20 Oct 2025 17:21:16 -0400
Message-ID: <375562.1760995276@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/375562.1760995276%40sss.pgh.pa.us>
Precedence: bulk

Dimitrios Apostolou <jimis@gmx.net> writes:
> Regarding the attached patch (rebased and edited commit message), it =

> basically replaces seek(up to 1MB forward) with read(). The 1MB number =

> comes a bit out of the top of my head. But tweaking it between 128KB and=
 =

> 1MB wouldn't really change anything, given that the block size is now =

> 128KB: The read() will always be chosen against the seek(). Do you know =

> of a real-world case with block sizes >128KB?

Yeah, with the recent changes I'd expect table data to pretty much
always consist of blocks around 128K, unless the table is smaller
than that of course.

I experimented with this patch locally and came away not too
impressed; it seems the results may be highly platform-dependent.

In the interests of having a common benchmark case that's easy to
replicate, here's precisely what I did:

Use non-assert build of current HEAD (4bea91f21 at the moment).

$ createdb bench
$ time pgbench -i -s 10000 bench

real    14m40.474s
user    1m26.717s
sys     0m5.045s
$ psql bench
...
bench=3D# create table zedtable(f1 int);
CREATE TABLE
bench=3D# insert into zedtable values(42);
INSERT 0 1
bench=3D# \q
$ time pg_dump -Fc --compress=3Dnone bench | cat >bench10000.dump

real    7m48.969s
user    0m36.334s
sys     1m35.209s

(At this -s value, the database occupies about 147G and the dump
file about 95G.  It's important the dump file not fit in RAM.)

$ time pg_restore -f /dev/null -t zedtable bench10000.dump

real    1m12.646s
user    0m0.355s
sys     0m5.083s

This compares rather favorably to "cat":

$ time cat bench10000.dump >/dev/null

real    3m6.627s
user    0m0.167s
sys     0m30.999s

I then applied your patch and repeated the restore run:

$ time pg_restore -f /dev/null -t zedtable bench10000.dump

real    2m39.138s
user    0m0.386s
sys     0m28.493s

So for me, the proposed patch actually makes it 2X slower.

Watching it with "iostat 1", I'm seeing about 40MB/s disk read
rate with HEAD, and 500MB/s with the patch; "cat" also shows
read rate around 500MB/s.  So yeah, we can saturate the disk
interface by doing all reads and no seeks, but that doesn't
net out faster.

I did this on a few-years-old Dell Precision 5820 workstation.
The specs for it are a bit vague about the disk subsystem:
  Storage Drive Controllers
  Integrated Intel AHCI SATA chipset controller (8x 6.0Gb/s), SW RAID 0,1,=
5,10
  Storage Drive
  2.5 1.92TB SATA AG Enterprise Solid State Drive
and hdparm isn't enormously helpful either:

ATA device, with non-removable media
	Model Number:       SSDSC2KB019T8R                          =

	Serial Number:      PHYF1291017A1P9DGN  =

	Firmware Revision:  XCV1DL69
	Media Serial Num:   =

	Media Manufacturer: =

	Transport:          Serial, ATA8-AST, SATA 1.0a, SATA II Extensions, SATA=
 Rev 2.5, SATA Rev 2.6, SATA Rev 3.0
Standards:
	Used: unknown (minor revision code 0x006d) =

	Supported: 10 9 8 7 6 5 =

	Likely used: 10

I'm running RHEL 8.10, file system is xfs.

So I find this a bit discouraging.  It's not clear why you're seeing a
win and I'm not, and it's even less clear whether there'd be enough
of a win across enough platforms to make it worth trying to engineer
a solution that helps many more people than it hurts.

			regards, tom lane