Re: Warn when parallel restoring a custom dump without data offsets
David Gilman <dgilman@gilslotd.com>
From: David Gilman <dgilman@gilslotd.com>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: David Gilman <davidgilman1@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers@lists.postgresql.org, thomas.munro@gmail.com
Date: 2020-05-25T18:54:29Z
Lists: pgsql-hackers
Attachments
- 0001-Remove-unused-seek-check-in-tar-dump-format.patch (text/x-diff)
Updated patches are attached, I ditched the gmail web interface so hopefully this works. Not mentioned in Justin's feedback: I dropped the extra sort in the test as it's no longer necessary. I also added a parallel dump -> parallel restore -> dump test run for the directory format to get some free test coverage. On Sat, May 23, 2020 at 05:47:51PM -0500, Justin Pryzby wrote: > I'm not sure this will be considered a bugfix, since the behavior is known. > Maybe eligible for backpatch though (?) I'm not familiar with how your release management works, but I'm personally fine with whatever version you can get it into. I urge you to try landing this as soon as possible. The minimum reproducible example in the test case is very minimal and I imagine all real world databases are going to trigger this. > I would have thought to mention the seeks() ; but it's true that the read()s now > grow quadratically. I did run a test, but I don't know how many objects would > be unreasonable or how many it'd take to show a problem. And I misunderstood how bad it was. I thought it was reading little header structs off the disk but it's actually reading the entire table (see _skipData). So you're quadratically rereading entire tables and thrashing your cache. Oops. > Maybe we should avoid fseeko(0, SEEK_SET) unless we need to wrap around after > EOF - I'm not sure. The seek location is already the location of the end of the last good object so just adding wraparound gives the good algorithmic performance from the technique in commit 42f70cd9c. I’ve gone ahead and implemented this. > Is it possible to dump to stdout (or pipe to cat or dd) to avoid a new option ? The underlying IPC::Run code seems to support piping in a cross-platform way. I am not a Perl master though and after spending an evening trying to get it to work I went with this approach. If you can put me in touch with anyone to help me out here I'd appreciate it. -- David Gilman :DG< https://gilslotd.com
Commits
-
Cope with data-offset-less archive files during out-of-order restores.
- 71e8e66f783c 13.0 landed
- f009591d6edd 14.0 landed
- 66232220ee6e 12.4 landed
-
Remove manual tracking of file position in pg_dump/pg_backup_custom.c.
- 447cf2f8e9dc 13.0 landed
- a8d0732ac2b5 14.0 landed
- 39a068ce6675 12.4 landed
-
Improve performance of tuple conversion map generation
- 42f70cd9c3db 12.0 cited
-
Fix pg_restore so parallel restore doesn't fail when the input file doesn't
- b779ea8a9a2d 9.0.0 cited