fix-out-of-order-ltsWriteBlock-1.patch

text/plain

Filename: fix-out-of-order-ltsWriteBlock-1.patch
Type: text/plain
Part: 0
Message: Re: Subtle bug in "Simplify tape block format" commit

Patch

Format: unified
File+
src/backend/utils/sort/logtape.c 34 11
diff --git a/src/backend/utils/sort/logtape.c b/src/backend/utils/sort/logtape.c
index 83e9424537..4a64c53af5 100644
--- a/src/backend/utils/sort/logtape.c
+++ b/src/backend/utils/sort/logtape.c
@@ -152,7 +152,17 @@ typedef struct LogicalTape
 struct LogicalTapeSet
 {
 	BufFile    *pfile;			/* underlying file for whole tape set */
-	long		nFileBlocks;	/* # of blocks used in underlying file */
+
+	/*
+	 * File size tracking. nBlocksWritten is the size of the underlying file,
+	 * in BLCKSZ blocks. nBlocksReserved is the number of blocks allocated by
+	 * ltsGetFreeBlock(), and it is always greater than or equal to
+	 * nBlocksWritten. Blocks between nBlocksReserved and nBlocksWritten are
+	 * blocks that have been allocated for a tape, but have not been written
+	 * to the underlying file yet.
+	 */
+	long		nBlocksReserved;	/* # of blocks allocated by ltsGetFreeBlock */
+	long		nBlocksWritten;		/* # of blocks used in underlying file */
 
 	/*
 	 * We store the numbers of recycled-and-available blocks in freeBlocks[].
@@ -187,21 +197,36 @@ static void ltsReleaseBlock(LogicalTapeSet *lts, long blocknum);
 /*
  * Write a block-sized buffer to the specified block of the underlying file.
  *
- * NB: should not attempt to write beyond current end of file (ie, create
- * "holes" in file), since BufFile doesn't allow that.  The first write pass
- * must write blocks sequentially.
- *
  * No need for an error return convention; we ereport() on any error.
  */
 static void
 ltsWriteBlock(LogicalTapeSet *lts, long blocknum, void *buffer)
 {
+	/*
+	 * BufFile does not support "holes", so if we're about to write a block
+	 * that's past the current end of file, fill the space between the
+	 * current end of file and the target block with zeros.
+	 */
+	while (blocknum > lts->nBlocksWritten)
+	{
+		char		zerobuf[BLCKSZ];
+
+		MemSet(zerobuf, 0, sizeof(zerobuf));
+
+		ltsWriteBlock(lts, lts->nBlocksWritten, zerobuf);
+	}
+
+	/* Write the requested block */
 	if (BufFileSeekBlock(lts->pfile, blocknum) != 0 ||
 		BufFileWrite(lts->pfile, buffer, BLCKSZ) != BLCKSZ)
 		ereport(ERROR,
 				(errcode_for_file_access(),
 				 errmsg("could not write block %ld of temporary file: %m",
 						blocknum)));
+
+	/* Update nBlocksWritten, if we extended the file */
+	if (blocknum == lts->nBlocksWritten)
+		lts->nBlocksWritten++;
 }
 
 /*
@@ -281,9 +306,6 @@ freeBlocks_cmp(const void *a, const void *b)
 
 /*
  * Select a currently unused block for writing to.
- *
- * NB: should only be called when writer is ready to write immediately,
- * to ensure that first write pass is sequential.
  */
 static long
 ltsGetFreeBlock(LogicalTapeSet *lts)
@@ -304,7 +326,7 @@ ltsGetFreeBlock(LogicalTapeSet *lts)
 		return lts->freeBlocks[--lts->nFreeBlocks];
 	}
 	else
-		return lts->nFileBlocks++;
+		return lts->nBlocksReserved++;
 }
 
 /*
@@ -360,7 +382,8 @@ LogicalTapeSetCreate(int ntapes)
 	lts = (LogicalTapeSet *) palloc(offsetof(LogicalTapeSet, tapes) +
 									ntapes * sizeof(LogicalTape));
 	lts->pfile = BufFileCreateTemp(false);
-	lts->nFileBlocks = 0L;
+	lts->nBlocksReserved = 0L;
+	lts->nBlocksWritten = 0L;
 	lts->forgetFreeSpace = false;
 	lts->blocksSorted = true;	/* a zero-length array is sorted ... */
 	lts->freeBlocksLen = 32;	/* reasonable initial guess */
@@ -858,5 +881,5 @@ LogicalTapeTell(LogicalTapeSet *lts, int tapenum,
 long
 LogicalTapeSetBlocks(LogicalTapeSet *lts)
 {
-	return lts->nFileBlocks;
+	return lts->nBlocksReserved;
 }