Fix Eval scripts defrag (broken 7.0 in RC1) (#10271)

Remove scripts defragger since it was broken since #10126 (released in 7.0 RC1).
would crash the server if defragger starts in a server that contains eval scripts.

In #10126 the global `lua_script` dict became a dict to a custom `luaScript` struct with an internal `robj`
in it instead of a generic `sds` -> `robj` dict. This means we need custom code to defrag it and since scripts
should never really cause much fragmentation it makes more sense to simply remove the defrag code for scripts.

src/defrag.c

@@ -128,6 +128,27 @@ robj *activeDefragStringOb(robj* ob, long *defragged) {
     return ret;
 }
 
+/* Defrag helper for lua scripts
+ *
+ * returns NULL in case the allocation wasn't moved.
+ * when it returns a non-null value, the old pointer was already released
+ * and should NOT be accessed. */
+luaScript *activeDefragLuaScript(luaScript *script, long *defragged) {
+    luaScript *ret = NULL;
+
+    /* try to defrag script struct */
+    if ((ret = activeDefragAlloc(script))) {
+        script = ret;
+        (*defragged)++;
+    }
+
+    /* try to defrag actual script object */
+    robj *ob = activeDefragStringOb(script->body, defragged);
+    if (ob) script->body = ob;
+
+    return ret;
+}
+
 /* Defrag helper for dictEntries to be used during dict iteration (called on
  * each step). Returns a stat of how many pointers were moved. */
 long dictIterDefragEntry(dictIterator *iter) {
@@ -256,6 +277,7 @@ long activeDefragZsetEntry(zset *zs, dictEntry *de) {
 #define DEFRAG_SDS_DICT_VAL_IS_SDS 1
 #define DEFRAG_SDS_DICT_VAL_IS_STROB 2
 #define DEFRAG_SDS_DICT_VAL_VOID_PTR 3
+#define DEFRAG_SDS_DICT_VAL_LUA_SCRIPT 4
 
 /* Defrag a dict with sds key and optional value (either ptr, sds or robj string) */
 long activeDefragSdsDict(dict* d, int val_type) {
@@ -280,6 +302,10 @@ long activeDefragSdsDict(dict* d, int val_type) {
             void *newptr, *ptr = dictGetVal(de);
             if ((newptr = activeDefragAlloc(ptr)))
                 de->v.val = newptr, defragged++;
+        } else if (val_type == DEFRAG_SDS_DICT_VAL_LUA_SCRIPT) {
+            void *newptr, *ptr = dictGetVal(de);
+            if ((newptr = activeDefragLuaScript(ptr, &defragged)))
+                de->v.val = newptr;
         }
         defragged += dictIterDefragEntry(di);
     }
@@ -939,7 +965,7 @@ long defragOtherGlobals() {
     /* there are many more pointers to defrag (e.g. client argv, output / aof buffers, etc.
      * but we assume most of these are short lived, we only need to defrag allocations
      * that remain static for a long time */
-    defragged += activeDefragSdsDict(evalScriptsDict(), DEFRAG_SDS_DICT_VAL_IS_STROB);
+    defragged += activeDefragSdsDict(evalScriptsDict(), DEFRAG_SDS_DICT_VAL_LUA_SCRIPT);
     defragged += moduleDefragGlobals();
     return defragged;
 }

src/eval.c

@@ -47,11 +47,6 @@ void ldbEnable(client *c);
 void evalGenericCommandWithDebugging(client *c, int evalsha);
 sds ldbCatStackValue(sds s, lua_State *lua, int idx);
 
-typedef struct luaScript {
-    uint64_t flags;
-    robj *body;
-} luaScript;
-
 static void dictLuaScriptDestructor(dict *d, void *val) {
     UNUSED(d);
     if (val == NULL) return; /* Lazy freeing will set value to NULL. */
@@ -63,7 +58,7 @@ static uint64_t dictStrCaseHash(const void *key) {
     return dictGenCaseHashFunction((unsigned char*)key, strlen((char*)key));
 }
 
-/* server.lua_scripts sha (as sds string) -> scripts (as robj) cache. */
+/* server.lua_scripts sha (as sds string) -> scripts (as luaScript) cache. */
 dictType shaScriptObjectDictType = {
         dictStrCaseHash,            /* hash function */
         NULL,                       /* key dup */

src/server.h

@@ -3066,6 +3066,11 @@ unsigned long evalMemory();
 dict* evalScriptsDict();
 unsigned long evalScriptsMemory();
 
+typedef struct luaScript {
+    uint64_t flags;
+    robj *body;
+} luaScript;
+
 /* Blocked clients */
 void processUnblockedClients(void);
 void blockClient(client *c, int btype);

tests/unit/memefficiency.tcl

@@ -157,6 +157,88 @@ start_server {tags {"defrag external:skip"} overrides {appendonly yes auto-aof-r
         }
         r config set appendonly no
         r config set key-load-delay 0
+        
+        test "Active defrag eval scripts" {
+            r flushdb
+            r script flush sync
+            r config resetstat
+            r config set hz 100
+            r config set activedefrag no
+            r config set active-defrag-threshold-lower 5
+            r config set active-defrag-cycle-min 65
+            r config set active-defrag-cycle-max 75
+            r config set active-defrag-ignore-bytes 300kb
+            r config set maxmemory 0
+            
+            set n 50000
+
+            # Populate memory with interleaving script-key pattern of same size
+            set dummy_script "--[string repeat x 200]\nreturn "
+            set rd [redis_deferring_client]
+            for {set j 0} {$j < $n} {incr j} {
+                set val "$dummy_script[format "%06d" $j]"
+                $rd script load $val
+                $rd set k$j $val
+            }
+            for {set j 0} {$j < $n} {incr j} {
+                $rd read ; # Discard script load replies
+                $rd read ; # Discard set replies
+            }
+            after 120 ;# serverCron only updates the info once in 100ms
+            if {$::verbose} {
+                puts "used [s allocator_allocated]"
+                puts "rss [s allocator_active]"
+                puts "frag [s allocator_frag_ratio]"
+                puts "frag_bytes [s allocator_frag_bytes]"
+            }                    
+            assert_lessthan [s allocator_frag_ratio] 1.05
+            
+            # Delete all the keys to create fragmentation
+            for {set j 0} {$j < $n} {incr j} { $rd del k$j }
+            for {set j 0} {$j < $n} {incr j} { $rd read } ; # Discard del replies
+            $rd close
+            after 120 ;# serverCron only updates the info once in 100ms
+            if {$::verbose} {
+                puts "used [s allocator_allocated]"
+                puts "rss [s allocator_active]"
+                puts "frag [s allocator_frag_ratio]"
+                puts "frag_bytes [s allocator_frag_bytes]"
+            }                    
+            assert_morethan [s allocator_frag_ratio] 1.4
+
+            catch {r config set activedefrag yes} e
+            if {[r config get activedefrag] eq "activedefrag yes"} {
+            
+                # wait for the active defrag to start working (decision once a second)
+                wait_for_condition 50 100 {
+                    [s active_defrag_running] ne 0
+                } else {
+                    fail "defrag not started."
+                }
+
+                # wait for the active defrag to stop working
+                wait_for_condition 500 100 {
+                    [s active_defrag_running] eq 0
+                } else {
+                    after 120 ;# serverCron only updates the info once in 100ms
+                    puts [r info memory]
+                    puts [r memory malloc-stats]
+                    fail "defrag didn't stop."
+                }
+
+                # test the the fragmentation is lower
+                after 120 ;# serverCron only updates the info once in 100ms
+                if {$::verbose} {
+                    puts "used [s allocator_allocated]"
+                    puts "rss [s allocator_active]"
+                    puts "frag [s allocator_frag_ratio]"
+                    puts "frag_bytes [s allocator_frag_bytes]"
+                }                    
+                assert_lessthan_equal [s allocator_frag_ratio] 1.05
+            }                
+            # Flush all script to make sure we don't crash after defragging them
+            r script flush sync
+        } {OK}
 
         test "Active defrag big keys" {
             r flushdb