Re: [HACKERS] Parallel tuplesort (for parallel B-Tree index creation)
Robert Haas <robertmhaas@gmail.com>
On Fri, Feb 2, 2018 at 10:26 PM, Peter Geoghegan <pg@bowt.ie> wrote: > My proposed commit message > has a full explanation of the Valgrind issue, which I won't repeat > here. Go read it before reading the rest of this e-mail. I'm going to paste the first two sentences of your proposed commit message in here for the convenience of other readers, since I want to reply to them. # LogicalTapeFreeze() may write out its first block when it is dirty but # not full, and then immediately read the first block back in from its # BufFile as a BLCKSZ-width block. This can only occur in rare cases # where next to no tuples were written out, which is only possible with # parallel external tuplesorts. So, if I understand correctly what you're saying here, valgrind is totally cool with us writing out an only-partially-initialized block to a disk file, but it's got a real problem with us reading that data back into the same memory space it already occupies. That's a little odd. I presume that it's common for the tail of the final block written to be uninitialized, but normally when we then go read block 0, that's some other, fully initialized block. It seems like it would be pretty easy to just suppress the useless read when we've already got the correct data, and I'd lean toward going that direction since it's a valid optimization anyway. But I'd like to hear some opinions from people who use and think about valgrind more than I do (Tom, Andres, Noah, ...?). > It might seem like my suppression is overly broad, or not broad > enough, since it essentially targets LogicalTapeFreeze(). I don't > think it is, though, because this can occur in two places within > LogicalTapeFreeze() -- it can occur in the place we actually saw the > issue on lousyjack, from the ltsReadBlock() call within > LogicalTapeFreeze(), as well as a second place -- when > BufFileExportShared() is called. I found that you have to tweak code > to prevent it happening in the first place before you'll see it happen > in the second place. I don't quite see how that would happen, because BufFileExportShared, at least AFAICS, doesn't touch the buffer? Unfortunately valgrind does not work at all on my laptop -- the server appears to start, but as soon as you try to connect, the whole thing dies with an error claiming that the startup process has failed. So I can't easily test this at the moment. I'll try to get it working, here or elsewhere, but thought I'd send the above reply first. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Commits
-
Support parallel btree index builds.
- 9da0cc35284b 11.0 landed
-
Report an ERROR if a parallel worker fails to start properly.
- 2badb5afb89c 11.0 cited
-
Transfer state pertaining to pending REINDEX operations to workers.
- 29d58fd3adae 11.0 landed
-
Add a barrier primitive for synchronizing backends.
- 1145acc70deb 11.0 cited
-
Allow DML commands that create tables to use parallel query.
- e9baa5e9fa14 11.0 cited
-
Refactor GetOldestXmin() to use flags
- af4b1a0869bd 10.0 cited
-
Fix regression in parallel planning against inheritance tables.
- 2609e91fcf9d 10.0 cited
-
Don't create "holes" in BufFiles, in the new logtape code.
- 7ac4a389a7db 10.0 cited
-
Simplify the code for logical tape read buffers.
- b75f467b6eec 10.0 cited
-
Fix excessive memory consumption in the new sort pre-reading code.
- b56fb691b003 10.0 cited
-
Implement binary heap replace-top operation in a smarter way.
- 24598337c8d2 10.0 cited
-
Cosmetic code cleanup in commands/extension.c.
- 25794e841e5b 10.0 cited
-
Speed up planner's scanning for parallel-query hazards.
- da1c91631e35 10.0 cited
-
Read from the same worker repeatedly until it returns no tuple.
- bc7fcab5e36b 9.6.0 cited
-
Improve tuplesort.c to support variable merge order. The original coding
- df700e6b4019 8.2.0 cited