From 9f971e7802d041573024eede7e0072372ef1d6e6 Mon Sep 17 00:00:00 2001 From: Siva R Date: Tue, 4 Sep 2018 19:19:16 +0000 Subject: [PATCH] Rewrite ginRedoRecompress to use a scratch space for processing replay actions instead of modifying contents on the actual page to avoid accidental overwrites of opaque data. --- src/backend/access/gin/ginxlog.c | 180 +++++++++++++++++++++------------------ 1 file changed, 98 insertions(+), 82 deletions(-) diff --git a/src/backend/access/gin/ginxlog.c b/src/backend/access/gin/ginxlog.c index 7515f8b..7ea798f 100644 --- a/src/backend/access/gin/ginxlog.c +++ b/src/backend/access/gin/ginxlog.c @@ -138,17 +138,21 @@ ginRedoRecompress(Page page, ginxlogRecompressDataLeaf *data) { int actionno; int segno; - GinPostingList *oldseg; + GinPostingList *segiter; + Pointer segmentcopybegin; Pointer segmentend; char *walbuf; int totalsize; + Pointer scratch; + Pointer scratchbegin; + bool modified; /* * If the page is in pre-9.4 format, convert to new format first. */ if (!GinPageIsCompressed(page)) { - ItemPointer uncompressed = (ItemPointer) GinDataPageGetData(page); + ItemPointer uncompressed = (ItemPointer)GinDataPageGetData(page); int nuncompressed = GinPageGetOpaque(page)->maxoff; int npacked; @@ -180,63 +184,77 @@ ginRedoRecompress(Page page, ginxlogRecompressDataLeaf *data) GinPageGetOpaque(page)->maxoff = InvalidOffsetNumber; } - oldseg = GinDataLeafPageGetPostingList(page); - segmentend = (Pointer) oldseg + GinDataLeafPageGetPostingListSize(page); - segno = 0; + scratch = palloc(GinDataPageMaxDataSize); + + segiter = GinDataLeafPageGetPostingList(page); + segmentend = (Pointer)segiter + GinDataLeafPageGetPostingListSize(page); + segmentcopybegin = segmentend; - walbuf = ((char *) data) + sizeof(ginxlogRecompressDataLeaf); + segno = 0; + modified = false; + scratchbegin = scratch; + walbuf = ((char *)data) + sizeof(ginxlogRecompressDataLeaf); for (actionno = 0; actionno < data->nactions; actionno++) { - uint8 a_segno = *((uint8 *) (walbuf++)); - uint8 a_action = *((uint8 *) (walbuf++)); + uint8 a_segno = *((uint8 *)(walbuf++)); + uint8 a_action = *((uint8 *)(walbuf++)); GinPostingList *newseg = NULL; int newsegsize = 0; ItemPointerData *items = NULL; uint16 nitems = 0; - ItemPointerData *olditems; - int nolditems; - ItemPointerData *newitems; - int nnewitems; - int segsize; - Pointer segptr; - int szleft; - - /* Extract all the information we need from the WAL record */ - if (a_action == GIN_SEGMENT_INSERT || - a_action == GIN_SEGMENT_REPLACE) - { - newseg = (GinPostingList *) walbuf; - newsegsize = SizeOfGinPostingList(newseg); - walbuf += SHORTALIGN(newsegsize); - } - - if (a_action == GIN_SEGMENT_ADDITEMS) - { - memcpy(&nitems, walbuf, sizeof(uint16)); - walbuf += sizeof(uint16); - items = (ItemPointerData *) walbuf; - walbuf += nitems * sizeof(ItemPointerData); - } /* Skip to the segment that this action concerns */ Assert(segno <= a_segno); while (segno < a_segno) { - oldseg = GinNextPostingListSegment(oldseg); + if (modified) + { + int cursegsize; + + /* Copy out the segment to scratch space */ + cursegsize = SizeOfGinPostingList(segiter); + Assert(scratch + cursegsize - scratchbegin <= GinDataPageMaxDataSize); + memcpy(scratch, segiter, cursegsize); + scratch += cursegsize; + } + + /* Move to next segment in the page */ + segiter = GinNextPostingListSegment(segiter); segno++; } /* - * ADDITEMS action is handled like REPLACE, but the new segment to - * replace the old one is reconstructed using the old segment from - * disk and the new items from the WAL record. + * Encountered first segment with an action. Every subsequent segment + * needs to be in scratch space. + */ + if (!modified) + { + modified = true; + segmentcopybegin = (Pointer)segiter; + } + + /* + * Positioned after the last existing segment. Only INSERTs expected + * here. */ + if ((Pointer)segiter == segmentend) + Assert(a_action == GIN_SEGMENT_INSERT); + + /* Process the action on the segment */ if (a_action == GIN_SEGMENT_ADDITEMS) { + ItemPointerData *olditems; + int nolditems; + ItemPointerData *newitems; + int nnewitems; int npacked; - olditems = ginPostingListDecode(oldseg, &nolditems); + memcpy(&nitems, walbuf, sizeof(uint16)); + walbuf += sizeof(uint16); + items = (ItemPointerData *)walbuf; + walbuf += nitems * sizeof(ItemPointerData); + olditems = ginPostingListDecode(segiter, &nolditems); newitems = ginMergeItemPointers(items, nitems, olditems, nolditems, &nnewitems); @@ -247,61 +265,59 @@ ginRedoRecompress(Page page, ginxlogRecompressDataLeaf *data) Assert(npacked == nnewitems); newsegsize = SizeOfGinPostingList(newseg); - a_action = GIN_SEGMENT_REPLACE; + memcpy(scratch, newseg, newsegsize); + scratch += newsegsize; } + else if (a_action == GIN_SEGMENT_INSERT || + a_action == GIN_SEGMENT_REPLACE) + { + newseg = (GinPostingList *)walbuf; + newsegsize = SizeOfGinPostingList(newseg); + walbuf += SHORTALIGN(SizeOfGinPostingList(newseg)); - segptr = (Pointer) oldseg; - if (segptr != segmentend) - segsize = SizeOfGinPostingList(oldseg); + /* Copy the new/replaced segment into scratch space */ + memcpy(scratch, newseg, newsegsize); + scratch += newsegsize; + } + else if (a_action == GIN_SEGMENT_DELETE) + { + /* No op */ + } else { - /* - * Positioned after the last existing segment. Only INSERTs - * expected here. - */ - Assert(a_action == GIN_SEGMENT_INSERT); - segsize = 0; + elog(ERROR, "unexpected GIN leaf action: %u", a_action); } - szleft = segmentend - segptr; - switch (a_action) + /* + * Advance to next segment on original page if the current segment + * action is not insert. + */ + if (a_action != GIN_SEGMENT_INSERT) { - case GIN_SEGMENT_DELETE: - memmove(segptr, segptr + segsize, szleft - segsize); - segmentend -= segsize; - - segno++; - break; - - case GIN_SEGMENT_INSERT: - /* make room for the new segment */ - memmove(segptr + newsegsize, segptr, szleft); - /* copy the new segment in place */ - memcpy(segptr, newseg, newsegsize); - segmentend += newsegsize; - segptr += newsegsize; - break; - - case GIN_SEGMENT_REPLACE: - /* shift the segments that follow */ - memmove(segptr + newsegsize, - segptr + segsize, - szleft - segsize); - /* copy the replacement segment in place */ - memcpy(segptr, newseg, newsegsize); - segmentend -= segsize; - segmentend += newsegsize; - segptr += newsegsize; - segno++; - break; - - default: - elog(ERROR, "unexpected GIN leaf action: %u", a_action); + segiter = GinNextPostingListSegment(segiter); + segno++; } - oldseg = (GinPostingList *) segptr; } - totalsize = segmentend - (Pointer) GinDataLeafPageGetPostingList(page); + /* Assert that there was some modification performed */ + Assert(modified); + + /* Copy out remainder of the page into scratch */ + if ((Pointer)segiter < segmentend) + memcpy(scratch, segiter, (segmentend - ((Pointer)segiter))); + + /* + * Check that the new total size is within the max data page size allowed. + * New total size is sum of scratch space size consumed and portion of + * page that was unmodified. + */ + totalsize = (scratch - scratchbegin) + (segmentcopybegin - (Pointer)GinDataLeafPageGetPostingList(page)); + Assert(totalsize <= GinDataPageMaxDataSize); + + /* Copy the items back onto the page */ + memcpy(segmentcopybegin, scratchbegin, (scratch - scratchbegin)); + + /* Set size on the page */ GinDataPageSetDataSize(page, totalsize); } -- 2.7.3.AMZN