Change LogicalTapeSetBlocks() to use nBlocksWritten.

jeff-davis · jeff-davis · commit 6e146a663536 · 2020-09-15T21:47:47.000-07:00
Previously, it was based on nBlocksAllocated to account for tapes with open write buffers that may not have made it to the BufFile yet. That was unnecessary, because callers do not need to get the number of blocks while a tape has an open write buffer; and it also conflicted with the preallocation logic added for HashAgg. Reviewed-by: Peter Geoghegan Discussion: https://postgr.es/m/ce5af05900fdbd0e9185747825a7423c48501964.camel@j-davis.com Backpatch-through: 13
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
@@ -2704,8 +2704,8 @@ agg_refill_hash_table(AggState *aggstate)
 
 	if (spill_initialized)
 	{
-		hash_agg_update_metrics(aggstate, true, spill.npartitions);
 		hashagg_spill_finish(aggstate, &spill, batch->setno);
+		hash_agg_update_metrics(aggstate, true, spill.npartitions);
 	}
 	else
 		hash_agg_update_metrics(aggstate, true, 0);
diff --git a/src/backend/utils/sort/logtape.c b/src/backend/utils/sort/logtape.c
@@ -1262,9 +1262,19 @@ LogicalTapeTell(LogicalTapeSet *lts, int tapenum,
 
 /*
  * Obtain total disk space currently used by a LogicalTapeSet, in blocks.
+ *
+ * This should not be called while there are open write buffers; otherwise it
+ * may not account for buffered data.
  */
 long
 LogicalTapeSetBlocks(LogicalTapeSet *lts)
 {
-	return lts->nBlocksAllocated - lts->nHoleBlocks;
+#ifdef USE_ASSERT_CHECKING
+	for (int i = 0; i < lts->nTapes; i++)
+	{
+		LogicalTape *lt = &lts->tapes[i];
+		Assert(!lt->writing || lt->buffer == NULL);
+	}
+#endif
+	return lts->nBlocksWritten - lts->nHoleBlocks;
 }

Original file line number	Diff line number	Diff line change
`@@ -2704,8 +2704,8 @@ agg_refill_hash_table(AggState *aggstate)`
`2704`	`2704`
`2705`	`2705`	`if (spill_initialized)`
`2706`	`2706`	`{`
`2707`		`- hash_agg_update_metrics(aggstate, true, spill.npartitions);`
`2708`	`2707`	`hashagg_spill_finish(aggstate, &spill, batch->setno);`
	`2708`	`+ hash_agg_update_metrics(aggstate, true, spill.npartitions);`
`2709`	`2709`	`}`
`2710`	`2710`	`else`
`2711`	`2711`	`hash_agg_update_metrics(aggstate, true, 0);`