Re: logtape.c stats don't account for unused "prefetched" block numbers

[Jeff Davis]

On Mon, 2020-09-14 at 20:48 -0700, Peter Geoghegan wrote:
> On Mon, Sep 14, 2020 at 8:07 PM Jeff Davis <pg...@j-davis.com> wrote:
> > Sure. Will backporting either patch into REL_13_STABLE now
> > interfere
> > with RC1 release in any way?
> 
> The RMT will discuss this.
> 
> It would help if there was a patch ready to go.

Attached. HashAgg seems to accurately reflect the filesize, as does
Sort.

Regards,
        Jeff Davis


diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index 28802e6588d..75e5bbf209d 100644
--- 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
index e904ce55185..71c8468b494 100644
--- 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 actively writing to tapes, 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;
 }