Re: Performance degradation on concurrent COPY into a single relation in PG16.
Andres Freund <andres@anarazel.de>
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Masahiko Sawada <sawada.mshk@gmail.com>, David Rowley <dgrowleyml@gmail.com>, Dean Rasheed <dean.a.rasheed@gmail.com>, Jakub Wartak <jakub.wartak@enterprisedb.com>, Andrew Dunstan <andrew@dunslane.net>, Peter Eisentraut <peter@eisentraut.org>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2023-10-14T02:33:44Z
Lists: pgsql-hackers
Hi, On 2023-10-13 11:30:35 -0700, Andres Freund wrote: > On 2023-10-13 10:39:10 -0700, Andres Freund wrote: > > On 2023-10-12 09:24:19 -0700, Andres Freund wrote: > > > I kind of had hoped somebody would comment on the approach. Given that nobody > > > has, I'll push the minimal fix of resetting the state in > > > ReleaseBulkInsertStatePin(), even though I think architecturally that's not > > > great. > > > > I spent some time working on a test that shows the problem more cheaply than > > the case upthread. I think it'd be desirable to have a test that's likely to > > catch an issue like this fairly quickly. We've had other problems in this > > realm before - there's only a single test that fails if I remove the > > ReleaseBulkInsertStatePin() call, and I don't think that's guaranteed at all. > > > > I'm a bit on the fence on how large to make the relation. For me the bug > > triggers when filling both relations up to the 3rd block, but how many rows > > that takes is somewhat dependent on space utilization on the page and stuff. > > > > Right now the test uses data/desc.data and ends up with 328kB and 312kB in two > > partitions. Alternatively I could make the test create a new file to load with > > copy that has fewer rows than data/desc.data - I didn't see another data file > > that works conveniently and has fewer rows. The copy is reasonably fast, even > > under something as expensive as rr (~60ms). So I'm inclined to just go with > > that? > > Patch with fix and test attached (0001). Pushed that. Greetings, Andres
Commits
-
Avoid edge case in pg_visibility test with small shared_buffers
- 408209d6a9ae 16.3 landed
- 3a4837fc809a 17.0 landed
-
Fix bulk table extension when copying into multiple partitions
- 0002feb82096 16.1 landed
- 22655aa23132 17.0 landed
-
hio: Take number of prior relation extensions into account
- 82a4edabd272 17.0 landed
- d37ab378b6e7 16.0 landed
-
Fix performance regression in pg_strtointNN_safe functions
- 4e2e75cd29eb 16.0 landed
- 3845577cb55e 17.0 landed
-
Fix performance problem with new COPY DEFAULT code
- c1308ce2d922 16.0 landed
- b635ac03e802 17.0 landed
-
hio: Use ExtendBufferedRelBy() to extend tables more efficiently
- 00d1e02be249 16.0 cited
-
Add VACUUM/ANALYZE BUFFER_USAGE_LIMIT option
- 1cbbee033857 16.0 cited