On 2017-11-27 22:25:12 +1300, Thomas Munro wrote:
> On Thu, Nov 23, 2017 at 12:36 AM, Thomas Munro
> <thomas.munro@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
