From 203988f5e788482b6445f03b3cbe4d3b47a9dc0d Mon Sep 17 00:00:00 2001
From: Hubert Zhang <hzhang@pivotal.io>
Date: Wed, 15 May 2019 10:04:00 +0000
Subject: [PATCH] Using growPenalty to replace growEnable in hashtable

When build hash table, we need to increase batch number
when spaceAllowed is reached. If the split process failed
(all the tuples fall into one batch and the other is empty)
then the split flag growEnable will be turned off in past.
Since later tuples may become diverse, and we'd better add
penalty instead of disable split.
---
 src/backend/executor/nodeHash.c | 28 ++++++++++++----------------
 src/include/executor/hashjoin.h |  2 +-
 2 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/src/backend/executor/nodeHash.c b/src/backend/executor/nodeHash.c
index 64eec91..f8cc8e9 100644
--- a/src/backend/executor/nodeHash.c
+++ b/src/backend/executor/nodeHash.c
@@ -493,7 +493,7 @@ ExecHashTableCreate(HashState *state, List *hashOperators, List *hashCollations,
 	hashtable->curbatch = 0;
 	hashtable->nbatch_original = nbatch;
 	hashtable->nbatch_outstart = nbatch;
-	hashtable->growEnabled = true;
+	hashtable->growPenalty = 0;
 	hashtable->totalTuples = 0;
 	hashtable->partialTuples = 0;
 	hashtable->skewTuples = 0;
@@ -892,10 +892,6 @@ ExecHashIncreaseNumBatches(HashJoinTable hashtable)
 	long		nfreed;
 	HashMemoryChunk oldchunks;
 
-	/* do nothing if we've decided to shut off growth */
-	if (!hashtable->growEnabled)
-		return;
-
 	/* safety check to avoid overflow */
 	if (oldnbatch > Min(INT_MAX / 2, MaxAllocSize / (sizeof(void *) * 2)))
 		return;
@@ -1030,19 +1026,19 @@ ExecHashIncreaseNumBatches(HashJoinTable hashtable)
 #endif
 
 	/*
-	 * If we dumped out either all or none of the tuples in the table, disable
-	 * further expansion of nbatch.  This situation implies that we have
-	 * enough tuples of identical hashvalues to overflow spaceAllowed.
-	 * Increasing nbatch will not fix it since there's no way to subdivide the
-	 * group any more finely. We have to just gut it out and hope the server
-	 * has enough RAM.
+	 * If we dumped out either all or none of the tuples in the table, punish
+	 * further expansion of nbatch. The penalty will increased the threshold
+	 * to recheck whether it is worth to increase nbatch next time.
+	 * In past, Postgres uses a simple flag growEnable to diskable expansion
+	 * of nbatch, this is not appropriate since data may with identical
+	 * hashvalues at the begining but would be diverse in future.
 	 */
 	if (nfreed == 0 || nfreed == ninmemory)
 	{
-		hashtable->growEnabled = false;
+		hashtable->growPenalty++;
 #ifdef HJDEBUG
-		printf("Hashjoin %p: disabling further increase of nbatch\n",
-			   hashtable);
+		printf("Hashjoin %p: further increase of nbatch with penalty: %d\n",
+			   hashtable, hashtable->growPenalty);
 #endif
 	}
 }
@@ -1655,7 +1651,7 @@ ExecHashTableInsert(HashJoinTable hashtable,
 			hashtable->spacePeak = hashtable->spaceUsed;
 		if (hashtable->spaceUsed +
 			hashtable->nbuckets_optimal * sizeof(HashJoinTuple)
-			> hashtable->spaceAllowed)
+			> hashtable->spaceAllowed * (hashtable->growPenalty + 1) )
 			ExecHashIncreaseNumBatches(hashtable);
 	}
 	else
@@ -2434,7 +2430,7 @@ ExecHashSkewTableInsert(HashJoinTable hashtable,
 		ExecHashRemoveNextSkewBucket(hashtable);
 
 	/* Check we are not over the total spaceAllowed, either */
-	if (hashtable->spaceUsed > hashtable->spaceAllowed)
+	if (hashtable->spaceUsed > hashtable->spaceAllowed * (hashtable->growPenalty + 1))
 		ExecHashIncreaseNumBatches(hashtable);
 
 	if (shouldFree)
diff --git a/src/include/executor/hashjoin.h b/src/include/executor/hashjoin.h
index 2c94b92..fbc721d 100644
--- a/src/include/executor/hashjoin.h
+++ b/src/include/executor/hashjoin.h
@@ -313,7 +313,7 @@ typedef struct HashJoinTableData
 	int			nbatch_original;	/* nbatch when we started inner scan */
 	int			nbatch_outstart;	/* nbatch when we started outer scan */
 
-	bool		growEnabled;	/* flag to shut off nbatch increases */
+	int			growPenalty;	/* nbatch increases penalty, default 0 */
 
 	double		totalTuples;	/* # tuples obtained from inner plan */
 	double		partialTuples;	/* # tuples obtained from inner plan by me */
-- 
1.8.3.1

