From 628c0dea1b12ae28ce44a8cfa638dc784fe1bb2b Mon Sep 17 00:00:00 2001 From: Binbin Date: Tue, 23 Jan 2024 20:26:33 +0800 Subject: [PATCH] Some cleanups around function (#12940) This PR did some cleanups around function: - drop the comment about Libraries Ctx, since we do have comment in functionsLibCtx, no need to maintain multiple copies. - remove outdated comment about the dropped Library description. - remove unused desc and code vars in functionExtractLibMetaData. - fix engines_nemory typo, changed it to engines_memory. - remove outdated comment about FUNCTION CREATE and FUNCTION INFO, FUNCTION CREATE was renamed to FUNCTION LOAD. - Check in initServer whether the return of functionsInit is OK. --- src/functions.c | 20 +++++--------------- src/functions.h | 15 ++++++++++----- src/lazyfree.c | 6 +++--- src/server.c | 5 ++++- tests/unit/functions.tcl | 3 ++- 5 files changed, 24 insertions(+), 25 deletions(-) diff --git a/src/functions.c b/src/functions.c index c858db975..646ef6308 100644 --- a/src/functions.c +++ b/src/functions.c @@ -118,10 +118,7 @@ dictType librariesDictType = { /* Dictionary of engines */ static dict *engines = NULL; -/* Libraries Ctx. - * Contains the dictionary that map a library name to library object, - * Contains the dictionary that map a function name to function object, - * and the cache memory used by all the functions */ +/* Libraries Ctx. */ static functionsLibCtx *curr_functions_lib_ctx = NULL; static size_t functionMallocSize(functionInfo *fi) { @@ -499,7 +496,6 @@ static void functionListReplyFlags(client *c, functionInfo *fi) { * Return general information about all the libraries: * * Library name * * The engine used to run the Library - * * Library description * * Functions list * * Library code (if WITHCODE is given) * @@ -681,7 +677,6 @@ void fcallroCommand(client *c) { * is saved separately with the following information: * * Library name * * Engine name - * * Library description * * Library code * RDB_OPCODE_FUNCTION2 is saved before each library to present * that the payload is a library. @@ -840,7 +835,6 @@ void functionHelpCommand(client *c) { " Return general information on all the libraries:", " * Library name", " * The engine used to run the Library", -" * Library description", " * Functions list", " * Library code (if WITHCODE is given)", " It also possible to get only function that matches a pattern using LIBRARYNAME argument.", @@ -894,9 +888,7 @@ static int functionsVerifyName(sds name) { int functionExtractLibMetaData(sds payload, functionsLibMataData *md, sds *err) { sds name = NULL; - sds desc = NULL; sds engine = NULL; - sds code = NULL; if (strncmp(payload, "#!", 2) != 0) { *err = sdsnew("Missing library metadata"); return C_ERR; @@ -948,9 +940,7 @@ int functionExtractLibMetaData(sds payload, functionsLibMataData *md, sds *err) error: if (name) sdsfree(name); - if (desc) sdsfree(desc); if (engine) sdsfree(engine); - if (code) sdsfree(code); sdsfreesplitres(parts, numparts); return C_ERR; } @@ -1084,15 +1074,15 @@ void functionLoadCommand(client *c) { unsigned long functionsMemory(void) { dictIterator *iter = dictGetIterator(engines); dictEntry *entry = NULL; - size_t engines_nemory = 0; + size_t engines_memory = 0; while ((entry = dictNext(iter))) { engineInfo *ei = dictGetVal(entry); engine *engine = ei->engine; - engines_nemory += engine->get_used_memory(engine->engine_ctx); + engines_memory += engine->get_used_memory(engine->engine_ctx); } dictReleaseIterator(iter); - return engines_nemory; + return engines_memory; } /* Return memory overhead of all the engines combine */ @@ -1119,7 +1109,7 @@ dict* functionsLibGet(void) { return curr_functions_lib_ctx->libraries; } -size_t functionsLibCtxfunctionsLen(functionsLibCtx *functions_ctx) { +size_t functionsLibCtxFunctionsLen(functionsLibCtx *functions_ctx) { return dictSize(functions_ctx->functions); } diff --git a/src/functions.h b/src/functions.h index 22af139eb..bb0ea4cb2 100644 --- a/src/functions.h +++ b/src/functions.h @@ -32,11 +32,16 @@ /* * functions.c unit provides the Redis Functions API: - * * FUNCTION CREATE - * * FUNCTION CALL + * * FUNCTION LOAD + * * FUNCTION LIST + * * FUNCTION CALL (FCALL and FCALL_RO) * * FUNCTION DELETE + * * FUNCTION STATS * * FUNCTION KILL - * * FUNCTION INFO + * * FUNCTION FLUSH + * * FUNCTION DUMP + * * FUNCTION RESTORE + * * FUNCTION HELP * * Also contains implementation for: * * Save/Load function from rdb @@ -59,7 +64,7 @@ typedef struct engine { * code - the library code * timeout - timeout for the library creation (0 for no timeout) * err - description of error (if occurred) - * returns NULL on error and set sds to be the error message */ + * returns C_ERR on error and set err to be the error message */ int (*create)(void *engine_ctx, functionLibInfo *li, sds code, size_t timeout, sds *err); /* Invoking a function, r_ctx is an opaque object (from engine POV). @@ -120,7 +125,7 @@ unsigned long functionsMemoryOverhead(void); unsigned long functionsNum(void); unsigned long functionsLibNum(void); dict* functionsLibGet(void); -size_t functionsLibCtxfunctionsLen(functionsLibCtx *functions_ctx); +size_t functionsLibCtxFunctionsLen(functionsLibCtx *functions_ctx); functionsLibCtx* functionsLibCtxGetCurrent(void); functionsLibCtx* functionsLibCtxCreate(void); void functionsLibCtxClearCurrent(int async); diff --git a/src/lazyfree.c b/src/lazyfree.c index 1b58bb78e..5e7a1cb34 100644 --- a/src/lazyfree.c +++ b/src/lazyfree.c @@ -55,7 +55,7 @@ void lazyFreeLuaScripts(void *args[]) { /* Release the functions ctx. */ void lazyFreeFunctionsCtx(void *args[]) { functionsLibCtx *functions_lib_ctx = args[0]; - size_t len = functionsLibCtxfunctionsLen(functions_lib_ctx); + size_t len = functionsLibCtxFunctionsLen(functions_lib_ctx); functionsLibCtxFree(functions_lib_ctx); atomicDecr(lazyfree_objects,len); atomicIncr(lazyfreed_objects,len); @@ -227,8 +227,8 @@ void freeLuaScriptsAsync(dict *lua_scripts) { /* Free functions ctx, if the functions ctx contains enough functions, free it in async way. */ void freeFunctionsAsync(functionsLibCtx *functions_lib_ctx) { - if (functionsLibCtxfunctionsLen(functions_lib_ctx) > LAZYFREE_THRESHOLD) { - atomicIncr(lazyfree_objects,functionsLibCtxfunctionsLen(functions_lib_ctx)); + if (functionsLibCtxFunctionsLen(functions_lib_ctx) > LAZYFREE_THRESHOLD) { + atomicIncr(lazyfree_objects,functionsLibCtxFunctionsLen(functions_lib_ctx)); bioCreateLazyFreeJob(lazyFreeFunctionsCtx,1,functions_lib_ctx); } else { functionsLibCtxFree(functions_lib_ctx); diff --git a/src/server.c b/src/server.c index 95da29605..bfabfa272 100644 --- a/src/server.c +++ b/src/server.c @@ -2879,7 +2879,10 @@ void initServer(void) { } scriptingInit(1); - functionsInit(); + if (functionsInit() == C_ERR) { + serverPanic("Functions initialization failed, check the server logs."); + exit(1); + } slowlogInit(); latencyMonitorInit(); diff --git a/tests/unit/functions.tcl b/tests/unit/functions.tcl index 9e8ec08f3..90d4bb801 100644 --- a/tests/unit/functions.tcl +++ b/tests/unit/functions.tcl @@ -64,7 +64,8 @@ start_server {tags {"scripting"}} { } {hello1} test {FUNCTION - test replace argument with failure keeps old libraries} { - catch {r function create LUA test REPLACE {error}} + catch {r function load REPLACE [get_function_code LUA test test {error}]} e + assert_match {ERR Error compiling function*} $e r fcall test 0 } {hello1}