Allow shrinking hashtables in low memory situations

src/debug.c

@@ -1028,6 +1028,7 @@ void debugCommand(client *c) {
         addReply(c, shared.ok);
     } else if (!strcasecmp(c->argv[1]->ptr, "dict-resizing") && c->argc == 3) {
         server.dict_resizing = atoi(c->argv[2]->ptr);
+        updateDictResizePolicy();
         addReply(c, shared.ok);
     } else if (!strcasecmp(c->argv[1]->ptr, "client-enforce-reply-list") && c->argc == 3) {
         server.debug_client_enforce_reply_list = atoi(c->argv[2]->ptr);

src/hashtable.c

@@ -614,14 +614,6 @@ static int resize(hashtable *ht, size_t min_capacity, int *malloc_failed) {
         return 0;
     }
 
-    if (ht->type->resizeAllowed) {
-        double fill_factor = (double)min_capacity / ((double)numBuckets(old_exp) * ENTRIES_PER_BUCKET);
-        if (fill_factor * 100 < MAX_FILL_PERCENT_HARD && !ht->type->resizeAllowed(alloc_size, fill_factor)) {
-            /* Resize callback says no. */
-            return 0;
-        }
-    }
-
     /* We can't resize if rehashing is already ongoing. Fast-forward ongoing
      * rehashing before we continue. This can happen only in exceptional
      * scenarios, such as when many insertions are made while rehashing is
@@ -633,6 +625,19 @@ static int resize(hashtable *ht, size_t min_capacity, int *malloc_failed) {
         }
     }
 
+    if (resize_policy == HASHTABLE_RESIZE_FORBID && ht->tables[0]) {
+        /* Refuse to resize if resizing is forbidden and we already have a primary table. */
+        return 0;
+    }
+    if (exp > old_exp && ht->type->resizeAllowed) {
+        /* If we're growing the table, let's check if the resizeAllowed callback allows the resize. */
+        double fill_factor = (double)min_capacity / ((double)numBuckets(old_exp) * ENTRIES_PER_BUCKET);
+        if (fill_factor * 100 < MAX_FILL_PERCENT_HARD && !ht->type->resizeAllowed(alloc_size, fill_factor)) {
+            /* Resize callback says no. */
+            return 0;
+        }
+    }
+
     /* Allocate the new hash table. */
     bucket *new_table;
     if (malloc_failed) {
@@ -1242,36 +1247,48 @@ int hashtableTryExpand(hashtable *ht, size_t size) {
 }
 
 /* Expanding is done automatically on insertion, but less eagerly if resize
- * policy is set to AVOID or FORBID. After restoring resize policy to ALLOW, you
- * may want to call hashtableExpandIfNeeded. Returns 1 if expanding, 0 if not
- * expanding. */
+ * policy is set to AVOID and not at all if set to FORBID.
+ * Returns 1 if expanding, 0 if not expanding. */
 int hashtableExpandIfNeeded(hashtable *ht) {
     size_t min_capacity = ht->used[0] + ht->used[1] + 1;
     size_t num_buckets = numBuckets(ht->bucket_exp[hashtableIsRehashing(ht) ? 1 : 0]);
     size_t current_capacity = num_buckets * ENTRIES_PER_BUCKET;
-    unsigned max_fill_percent = resize_policy == HASHTABLE_RESIZE_AVOID ? MAX_FILL_PERCENT_HARD : MAX_FILL_PERCENT_SOFT;
+    unsigned max_fill_percent = resize_policy == HASHTABLE_RESIZE_ALLOW ? MAX_FILL_PERCENT_SOFT : MAX_FILL_PERCENT_HARD;
     if (min_capacity * 100 <= current_capacity * max_fill_percent) {
         return 0;
     }
     return resize(ht, min_capacity, NULL);
 }
 
 /* Shrinking is done automatically on deletion, but less eagerly if resize
- * policy is set to AVOID and not at all if set to FORBID. After restoring
- * resize policy to ALLOW, you may want to call hashtableShrinkIfNeeded. */
+ * policy is set to AVOID and not at all if set to FORBID.
+ * Returns 1 if shrinking, 0 if not shrinking. */
 int hashtableShrinkIfNeeded(hashtable *ht) {
     /* Don't shrink if rehashing is already in progress. */
-    if (hashtableIsRehashing(ht) || resize_policy == HASHTABLE_RESIZE_FORBID || ht->pause_auto_shrink) {
+    if (hashtableIsRehashing(ht) || ht->pause_auto_shrink) {
         return 0;
     }
     size_t current_capacity = numBuckets(ht->bucket_exp[0]) * ENTRIES_PER_BUCKET;
-    unsigned min_fill_percent = resize_policy == HASHTABLE_RESIZE_AVOID ? MIN_FILL_PERCENT_HARD : MIN_FILL_PERCENT_SOFT;
+    unsigned min_fill_percent = resize_policy == HASHTABLE_RESIZE_ALLOW ? MIN_FILL_PERCENT_SOFT : MIN_FILL_PERCENT_HARD;
     if (ht->used[0] * 100 > current_capacity * min_fill_percent) {
         return 0;
     }
     return resize(ht, ht->used[0], NULL);
 }
 
+/* Resizes the hashtable to an optimal size, based on the current number of
+ * entries. This is a convenience function that first tries to shrink the table
+ * if needed, and then expands it if needed. After restoring resize policy
+ * to ALLOW, you may want to call hashtableShrinkIfNeeded.
+ * Returns 1 if resizing was performed, 0 otherwise. */
+int hashtableRightsizeIfNeeded(hashtable *ht) {
+    int ret = hashtableShrinkIfNeeded(ht);
+    if (!ret) {
+        ret = hashtableExpandIfNeeded(ht);
+    }
+    return ret;
+}
+
 /* Defragment the main allocations of the hashtable by reallocating them. The
  * provided defragfn callback should either return NULL (if reallocation is not
  * necessary) or reallocate the memory like realloc() would do.

src/hashtable.h

@@ -129,6 +129,7 @@ int hashtableExpand(hashtable *ht, size_t size);
 int hashtableTryExpand(hashtable *ht, size_t size);
 int hashtableExpandIfNeeded(hashtable *ht);
 int hashtableShrinkIfNeeded(hashtable *ht);
+int hashtableRightsizeIfNeeded(hashtable *ht);
 hashtable *hashtableDefragTables(hashtable *ht, void *(*defragfn)(void *));
 void dismissHashtable(hashtable *ht);
 

src/kvstore.c

@@ -631,9 +631,7 @@ void kvstoreTryResizeHashtables(kvstore *kvs, int limit) {
     for (int i = 0; i < limit; i++) {
         int didx = kvs->resize_cursor;
         hashtable *ht = kvstoreGetHashtable(kvs, didx);
-        if (ht && !hashtableShrinkIfNeeded(ht)) {
-            hashtableExpandIfNeeded(ht);
-        }
+        if (ht) hashtableRightsizeIfNeeded(ht);
         kvs->resize_cursor = (didx + 1) % kvs->num_hashtables;
     }
 }

src/server.c

@@ -522,9 +522,6 @@ uint64_t dictEncObjHash(const void *key) {
 int hashtableResizeAllowed(size_t moreMem, double usedRatio) {
     UNUSED(usedRatio);
 
-    /* For debug purposes, not allowed to be resized. */
-    if (!server.dict_resizing) return 0;
-
     /* Avoid resizing over max memory. */
     return !overMaxmemoryAfterAlloc(moreMem);
 }
@@ -811,7 +808,7 @@ hashtableType clientHashtableType = {
  * for dict.c to resize or rehash the tables accordingly to the fact we have an
  * active fork child running. */
 void updateDictResizePolicy(void) {
-    if (server.in_fork_child != CHILD_TYPE_NONE) {
+    if (server.in_fork_child != CHILD_TYPE_NONE || !server.dict_resizing) {
         dictSetResizeEnabled(DICT_RESIZE_FORBID);
         hashtableSetResizePolicy(HASHTABLE_RESIZE_FORBID);
     } else if (hasActiveChildProcess()) {

tests/unit/expire.tcl

@@ -1037,6 +1037,13 @@ start_cluster 1 0 {tags {"expire external:skip cluster"}} {
         # Enable resizing
         r debug dict-resizing 1
 
+        # Wait for ongoing rehashing to complete, if any
+        wait_for_condition 100 50 {
+            [dict get [r memory stats] db.dict.rehashing.count] == 0
+        } else {
+            fail "Active rehashing didn't finish"
+        }
+
         # put some data into slot 12182 and trigger the resize
         # by deleting it to trigger shrink
         r psetex "{foo}0" 500 a