On 2017-11-27 22:25:12 +1300, Thomas Munro wrote: > On Thu, Nov 23, 2017 at 12:36 AM, Thomas Munro > <thomas.mu...@enterprisedb.com> wrote: > > Here's a new patch set with responses to the last batch of review comments.
Looking at 0004-Add-shared-tuplestores.patch Comments: - I'd rename mutex to lock. Seems quite possible that we end up with shared lockers too. - Expand "Simple mechanism for sharing tuples between backends." intro comment - that doesn't really seem like a meaningful description of the mechanism. Should probably mention that it's similar to tuplestores etc... - I'm still concerned about the chunking mechanism. How about this sketch of an alternative solution? Chunks are always the same length. To avoid having to read the length from disk while holding a lock, introduce continuation chunks which store the amount of space needed by the overlarge tuple at the start. The reading process stays largely the same, except that if a backend reads a chunk that's a continuation, it reads the length of the continuation and skips ahead. That could mean that more than one backend read continuation chunks, but the window is small and there's normally not goign to be that many huge tuples (otherwise things are slow anyway). - why are we using a separate hardcoded 32 for sts names? Why not just go for NAMEDATALEN or such? - I'd replace most of the "should's" in comments with "need". Greetings, Andres Freund
