From 118c9bb8d14ebfcbb4a0a4af6f029d8d8fe46ad6 Mon Sep 17 00:00:00 2001 From: Tom Lane <tgl@sss.pgh.pa.us> Date: Tue, 4 Aug 2015 18:18:46 -0400 Subject: [PATCH] Fix bogus "out of memory" reports in tuplestore.c. The tuplesort/tuplestore memory management logic assumed that the chunk allocation overhead for its memtuples array could not increase when increasing the array size. This is and always was true for tuplesort, but we (I, I think) blindly copied that logic into tuplestore.c without noticing that the assumption failed to hold for the much smaller array elements used by tuplestore. Given rather small work_mem, this could result in an improper complaint about "unexpected out-of-memory situation", as reported by Brent DeSpain in bug #13530. The easiest way to fix this is just to increase tuplestore's initial array size so that the assumption holds. Rather than relying on magic constants, though, let's export a #define from aset.c that represents the safe allocation threshold, and make tuplestore's calculation depend on that. Do the same in tuplesort.c to keep the logic looking parallel, even though tuplesort.c isn't actually at risk at present. This will keep us from breaking it if we ever muck with the allocation parameters in aset.c. Back-patch to all supported versions. The error message doesn't occur pre-9.3, not so much because the problem can't happen as because the pre-9.3 tuplestore code neglected to check for it. (The chance of trouble is a great deal larger as of 9.3, though, due to changes in the array-size-increasing strategy.) However, allowing LACKMEM() to become true unexpectedly could still result in less-than-desirable behavior, so let's patch it all the way back. --- src/backend/utils/mmgr/aset.c | 11 ++++++++--- src/backend/utils/sort/tuplesort.c | 19 +++++++++++++------ src/backend/utils/sort/tuplestore.c | 19 +++++++++++++------ src/include/utils/memutils.h | 8 ++++++++ 4 files changed, 42 insertions(+), 15 deletions(-) diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c index 743455e4bcc..9dfdc2e87e0 100644 --- a/src/backend/utils/mmgr/aset.c +++ b/src/backend/utils/mmgr/aset.c @@ -112,9 +112,9 @@ * * With the current parameters, request sizes up to 8K are treated as chunks, * larger requests go into dedicated blocks. Change ALLOCSET_NUM_FREELISTS - * to adjust the boundary point. (But in contexts with small maxBlockSize, - * we may set the allocChunkLimit to less than 8K, so as to avoid space - * wastage.) + * to adjust the boundary point; and adjust ALLOCSET_SEPARATE_THRESHOLD in + * memutils.h to agree. (Note: in contexts with small maxBlockSize, we may + * set the allocChunkLimit to less than 8K, so as to avoid space wastage.) *-------------------- */ @@ -476,7 +476,12 @@ AllocSetContextCreate(MemoryContext parent, * We have to have allocChunkLimit a power of two, because the requested * and actually-allocated sizes of any chunk must be on the same side of * the limit, else we get confused about whether the chunk is "big". + * + * Also, allocChunkLimit must not exceed ALLOCSET_SEPARATE_THRESHOLD. */ + StaticAssertStmt(ALLOC_CHUNK_LIMIT == ALLOCSET_SEPARATE_THRESHOLD, + "ALLOC_CHUNK_LIMIT != ALLOCSET_SEPARATE_THRESHOLD"); + context->allocChunkLimit = ALLOC_CHUNK_LIMIT; while ((Size) (context->allocChunkLimit + ALLOC_CHUNKHDRSZ) > (Size) ((maxBlockSize - ALLOC_BLOCKHDRSZ) / ALLOC_CHUNK_FRACTION)) diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c index e78a51fe228..18cb046f198 100644 --- a/src/backend/utils/sort/tuplesort.c +++ b/src/backend/utils/sort/tuplesort.c @@ -571,7 +571,14 @@ tuplesort_begin_common(int workMem, bool randomAccess) state->tapeset = NULL; state->memtupcount = 0; - state->memtupsize = 1024; /* initial guess */ + + /* + * Initial size of array must be more than ALLOCSET_SEPARATE_THRESHOLD; + * see comments in grow_memtuples(). + */ + state->memtupsize = Max(1024, + ALLOCSET_SEPARATE_THRESHOLD / sizeof(SortTuple) + 1); + state->growmemtuples = true; state->memtuples = (SortTuple *) palloc(state->memtupsize * sizeof(SortTuple)); @@ -1064,10 +1071,10 @@ grow_memtuples(Tuplesortstate *state) * never generate a dangerous request, but to be safe, check explicitly * that the array growth fits within availMem. (We could still cause * LACKMEM if the memory chunk overhead associated with the memtuples - * array were to increase. That shouldn't happen with any sane value of - * allowedMem, because at any array size large enough to risk LACKMEM, - * palloc would be treating both old and new arrays as separate chunks. - * But we'll check LACKMEM explicitly below just in case.) + * array were to increase. That shouldn't happen because we chose the + * initial array size large enough to ensure that palloc will be treating + * both old and new arrays as separate chunks. But we'll check LACKMEM + * explicitly below just in case.) */ if (state->availMem < (int64) ((newmemtupsize - memtupsize) * sizeof(SortTuple))) goto noalloc; @@ -1080,7 +1087,7 @@ grow_memtuples(Tuplesortstate *state) state->memtupsize * sizeof(SortTuple)); USEMEM(state, GetMemoryChunkSpace(state->memtuples)); if (LACKMEM(state)) - elog(ERROR, "unexpected out-of-memory situation during sort"); + elog(ERROR, "unexpected out-of-memory situation in tuplesort"); return true; noalloc: diff --git a/src/backend/utils/sort/tuplestore.c b/src/backend/utils/sort/tuplestore.c index 1d6fe869944..c5f7f3d01db 100644 --- a/src/backend/utils/sort/tuplestore.c +++ b/src/backend/utils/sort/tuplestore.c @@ -265,7 +265,14 @@ tuplestore_begin_common(int eflags, bool interXact, int maxKBytes) state->memtupdeleted = 0; state->memtupcount = 0; - state->memtupsize = 1024; /* initial guess */ + + /* + * Initial size of array must be more than ALLOCSET_SEPARATE_THRESHOLD; + * see comments in grow_memtuples(). + */ + state->memtupsize = Max(16384 / sizeof(void *), + ALLOCSET_SEPARATE_THRESHOLD / sizeof(void *) + 1); + state->growmemtuples = true; state->memtuples = (void **) palloc(state->memtupsize * sizeof(void *)); @@ -639,10 +646,10 @@ grow_memtuples(Tuplestorestate *state) * never generate a dangerous request, but to be safe, check explicitly * that the array growth fits within availMem. (We could still cause * LACKMEM if the memory chunk overhead associated with the memtuples - * array were to increase. That shouldn't happen with any sane value of - * allowedMem, because at any array size large enough to risk LACKMEM, - * palloc would be treating both old and new arrays as separate chunks. - * But we'll check LACKMEM explicitly below just in case.) + * array were to increase. That shouldn't happen because we chose the + * initial array size large enough to ensure that palloc will be treating + * both old and new arrays as separate chunks. But we'll check LACKMEM + * explicitly below just in case.) */ if (state->availMem < (int64) ((newmemtupsize - memtupsize) * sizeof(void *))) goto noalloc; @@ -655,7 +662,7 @@ grow_memtuples(Tuplestorestate *state) state->memtupsize * sizeof(void *)); USEMEM(state, GetMemoryChunkSpace(state->memtuples)); if (LACKMEM(state)) - elog(ERROR, "unexpected out-of-memory situation during sort"); + elog(ERROR, "unexpected out-of-memory situation in tuplestore"); return true; noalloc: diff --git a/src/include/utils/memutils.h b/src/include/utils/memutils.h index 59d0aecfbbc..9f499d11804 100644 --- a/src/include/utils/memutils.h +++ b/src/include/utils/memutils.h @@ -145,4 +145,12 @@ extern MemoryContext AllocSetContextCreate(MemoryContext parent, #define ALLOCSET_SMALL_INITSIZE (1 * 1024) #define ALLOCSET_SMALL_MAXSIZE (8 * 1024) +/* + * Threshold above which a request in an AllocSet context is certain to be + * allocated separately (and thereby have constant allocation overhead). + * Few callers should be interested in this, but tuplesort/tuplestore need + * to know it. + */ +#define ALLOCSET_SEPARATE_THRESHOLD 8192 + #endif /* MEMUTILS_H */ -- GitLab