On 8/29/22 17:27, Tomas Vondra wrote:
> ...
>
> I suspect it's a pre-existing bug in Slab allocator, because it does this:
>
> #define SlabBlockGetChunk(slab, block, idx) \
> ((MemoryChunk *) ((char *) (block) + sizeof(SlabBlock) \
> + (idx * slab->fullChunkSize)))
>
> and SlabBlock is only 20B, i.e. not a multiple of 8B. Which would mean
> that even if we allocate block and size the chunks carefully (with all
> the MAXALIGN things), we ultimately slice the block incorrectly.
>
The attached patch seems to fix the issue for me - at least it seems
like that. This probably will need to get backpatched, I guess. Maybe we
should add an assert to MemoryChunkGetPointer to check alignment?
> This would explain the 4B difference I reported before, I think. But I'm
> just as astonished we got this far in the tests - regular regression
> tests don't do much logical decoding, and we only use slab for changes,
> but I see the failure in 006 test in src/test/recovery, so the first
> five completed fine.
>
I got confused - the first 5 tests in src/test/recovery don't do any
logical decoding, so it's not surprising it's the 006 that fails.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c
index ae1a735b8c..67fbede836 100644
--- a/src/backend/utils/mmgr/slab.c
+++ b/src/backend/utils/mmgr/slab.c
@@ -102,10 +102,10 @@ typedef struct SlabBlock
#define SlabChunkGetPointer(chk) \
((void *)(((char *)(chk)) + sizeof(MemoryChunk)))
#define SlabBlockGetChunk(slab, block, idx) \
- ((MemoryChunk *) ((char *) (block) + sizeof(SlabBlock) \
+ ((MemoryChunk *) ((char *) (block) + MAXALIGN(sizeof(SlabBlock)) \
+ (idx * slab->fullChunkSize)))
#define SlabBlockStart(block) \
- ((char *) block + sizeof(SlabBlock))
+ ((char *) block + MAXALIGN(sizeof(SlabBlock)))
#define SlabChunkIndex(slab, block, chunk) \
(((char *) chunk - SlabBlockStart(block)) / slab->fullChunkSize)
@@ -146,12 +146,12 @@ SlabContextCreate(MemoryContext parent,
fullChunkSize = Slab_CHUNKHDRSZ + MAXALIGN(chunkSize);
/* Make sure the block can store at least one chunk. */
- if (blockSize < fullChunkSize + sizeof(SlabBlock))
+ if (blockSize < fullChunkSize + MAXALIGN(sizeof(SlabBlock)))
elog(ERROR, "block size %zu for slab is too small for %zu chunks",
blockSize, chunkSize);
/* Compute maximum number of chunks per block */
- chunksPerBlock = (blockSize - sizeof(SlabBlock)) / fullChunkSize;
+ chunksPerBlock = (blockSize - MAXALIGN(sizeof(SlabBlock))) / fullChunkSize;
/* The freelist starts with 0, ends with chunksPerBlock. */
freelistSize = sizeof(dlist_head) * (chunksPerBlock + 1);
