[MOD-8039] Reorganize stats files by meiravgri · Pull Request #5329 · RediSearch/RediSearch

meiravgri · 2024-12-09T16:24:58Z

This PR reorganizes the files related to the module info to improve structure and readability.

Elaborate `info` dir

Introduce `info_redis.*` files

these files are responsible to aggregate and output all the shard info to INFO MODULES command.

Move RS_moduleInfoFunc implementation from module-init.c
This function is registered via RedisModuleAPI to be called upon INFO MODULES (or INFO EVERYTHING)
Implement each info section in a static function to enhance readability
Relocate *_AddToInfo
- Moved from global_stats.c
  FieldsGlobalStats_AddToInfo
  TotalGlobalStats_Queries_AddToInfo
  DialectsGlobalStats_AddToInfo
  IndexesGlobalStats_AddToInfo
- Moved from config.c:
  RSConfig_AddToInfo

Introduce `indexes_info.*` files

Files dedicated to aggregating shard indexes information.

Move structs from info_command.h to indexes_info.*
Rename structs:
- TotalSpecsInfo -> TotalIndexesInfo
- TotalSpecsFieldInfo -> TotalIndexesFieldsInfo
Exposes IndexesInfo_TotalInfo. The implementation was moved from redisearch_api.c::RediSearch_TotalInfo
redisearch_api.c::RediSearch_TotalInfo now calls` index_info::IndexesInfo_TotalInfo

global_stats.* files

move to info/ dir
Relocat all *_AddToInfo functions:
Functions responsible for writing stats to INFO MODULES have been moved to INFO_MODULES.c.
introduce QueriesGlobalStats to hold queries information
introduce TotalGlobalStats_GetQueryStats
Atomically reads and copies query stats into a QueriesGlobalStats struct, which is returned to the caller.

info_command.* files

moved to info dir
move structs to indexes_info

Additional changes

util/units.h

Contains units macro definitions

Move CLOCKS_PER_MILLISEC from profile.c and nfo_command.h to util/units.h
MEMORY_MB

new test

added a unit test for redisearch_API::RediSearch_TotalInfo

responsible to expose all aggregated shard stats to INFO command move RS_moduleInfoFunc from module-init to move all add to info functions to a dedicated file: from global_stats.c FieldsGlobalStats_AddToInfo TotalGlobalStats_Queries_AddToInfo DialectsGlobalStats_AddToInfo IndexesGlobalStats_AddToInfo from config.c: RSConfig_AddToInfo introduce FieldsGlobalStats_GetIndexError to expose fields errors move queries stats from TotalGlobalStats to seperate struct QueriesGlobalStats

…indexes_info IndexesInfo_TotalInfo (orignally rediSearchAPI::RediSearch_TotalInfo) calls IndexSpec_TotalMemUsage instead of rediSearchAPI::RediSearch_TotalMemUsage introduce indexes_info rename TotalSpecsFieldInfo and TotalSpecsInfo: specs->indexes move TotalIndexesFieldsInfo and TotalIndexesInfo from info_command to indexes_info move global stats to info dir move info_command to info dir introduce util/units. A file for all macros handling shared units conversion remove CLOCKS_PER_MILLISEC from profile.h, include util/units.h instead add a cpp test for llapi indexes info

…INFO

meiravgri · 2024-12-10T14:41:56Z

src/config.c

  return rc;
 }

-void RSConfig_AddToInfo(RedisModuleInfoCtx *ctx) {


mvoed to info/INFO_MODULES.c

meiravgri · 2024-12-10T14:43:30Z

src/info/global_stats.c

@@ -0,0 +1,93 @@
+/*


moved all *_addToInfo to info/INFO_MODULES.c

meiravgri · 2024-12-10T14:44:36Z

src/info/indexes_info.c

+#include "util/dict.h"
+#include "spec.h"
+
+TotalIndexesInfo IndexesInfo_TotalInfo() {


moved from redisearch_api

meiravgri · 2024-12-10T14:45:20Z

src/info/indexes_info.c

+    }
+    // Lock for read
+    pthread_rwlock_rdlock(&sp->rwlock);
+    size_t cur_mem = IndexSpec_TotalMemUsage(sp, 0, 0, 0);


replaces RediSearch_TotalMemUsage (that calls IndexSpec_TotalMemUsage)

meiravgri · 2024-12-10T14:46:29Z

src/info/indexes_info.h

  size_t total_active_writes;          // Total number of active writes
  size_t total_active_queries;         // Total number of active queries (reads)
-} TotalSpecsInfo;
+} TotalIndexesInfo;


both struct were moved here from info_command

meiravgri · 2024-12-10T14:46:47Z

src/info/info_command.h

+#ifdef __cplusplus
+extern "C" {
+#endif
+


moved structs to indexes_info

meiravgri · 2024-12-10T14:47:07Z

src/module-init/module-init.c

  return REDISMODULE_OK;
 }

-#define MEMORY_HUMAN(x) ((x) / (double)(1024 * 1024))


moved macro definition to util/units

When are we dropping this misleading macro? we said we don't want it

meiravgri · 2024-12-10T14:47:18Z

src/module-init/module-init.c


-#define MEMORY_HUMAN(x) ((x) / (double)(1024 * 1024))
-
-void RS_moduleInfoFunc(RedisModuleInfoCtx *ctx, int for_crash_report) {


moved to INFO_MODULES

meiravgri · 2024-12-10T14:47:35Z

src/profile.h

 #include "value.h"
 #include "aggregate/aggregate.h"

-#define CLOCKS_PER_MILLISEC (CLOCKS_PER_SEC / 1000)


moved to "util/units.h"

meiravgri · 2024-12-10T14:47:50Z

src/redisearch_api.c

 }

 // Collect statistics of all the currently existing indexes
-TotalSpecsInfo RediSearch_TotalInfo(void) {


moved to info/indexes_info

meiravgri · 2024-12-10T15:07:31Z

src/info/global_stats.h

+ * Safely reads and returns a copy of the global queries stats.
 */
-void DialectsGlobalStats_AddToInfo(RedisModuleInfoCtx *ctx);
+QueriesGlobalStats TotalGlobalStats_GetQueryStats();


atomically reads queries stats

codecov · 2024-12-10T15:08:00Z

Codecov Report

Attention: Patch coverage is 98.51301% with 4 lines in your changes missing coverage. Please review.

Project coverage is 86.50%. Comparing base (98b342c) to head (dafebec).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
src/info/info_redis.c	98.27%	3 Missing ⚠️
src/info/indexes_info.c	97.36%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5329      +/-   ##
==========================================
+ Coverage   86.49%   86.50%   +0.01%     
==========================================
  Files         193      195       +2     
  Lines       34773    34817      +44     
==========================================
+ Hits        30076    30120      +44     
  Misses       4697     4697

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

remove unused header from module-init

src/info/info_command.h

GuyAv46 · 2024-12-11T14:15:33Z

src/module-init/module-init.c

  return REDISMODULE_OK;
 }

-#define MEMORY_HUMAN(x) ((x) / (double)(1024 * 1024))


When are we dropping this misleading macro? we said we don't want it

src/util/units.h

GuyAv46

Also, consider renaming INFO_MODULES.* to a name in lower case.

rename INFO_MODULES->info_redis rename MEMORY_HUMAN->MEMORY_MB

redisearch-backport-pull-request · 2024-12-15T14:39:50Z

Backport failed for 2.10, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 2.10
git worktree add -d .worktree/backport-5329-to-2.10 origin/2.10
cd .worktree/backport-5329-to-2.10
git switch --create backport-5329-to-2.10
git cherry-pick -x 175d78713598207192fd4e26b41fd88f4a32ae83

redisearch-backport-pull-request · 2024-12-15T14:39:53Z

Successfully created backport PR for 8.0:

[8.0] [MOD-8039] Reorganize stats files #5351

* new file : info/INFO_MODULES responsible to expose all aggregated shard stats to INFO command move RS_moduleInfoFunc from module-init to move all add to info functions to a dedicated file: from global_stats.c FieldsGlobalStats_AddToInfo TotalGlobalStats_Queries_AddToInfo DialectsGlobalStats_AddToInfo IndexesGlobalStats_AddToInfo from config.c: RSConfig_AddToInfo introduce FieldsGlobalStats_GetIndexError to expose fields errors move queries stats from TotalGlobalStats to seperate struct QueriesGlobalStats * rediSearchAPI::RediSearch_TotalInfo calls IndexesInfo_TotalInfo from indexes_info IndexesInfo_TotalInfo (orignally rediSearchAPI::RediSearch_TotalInfo) calls IndexSpec_TotalMemUsage instead of rediSearchAPI::RediSearch_TotalMemUsage introduce indexes_info rename TotalSpecsFieldInfo and TotalSpecsInfo: specs->indexes move TotalIndexesFieldsInfo and TotalIndexesInfo from info_command to indexes_info move global stats to info dir move info_command to info dir introduce util/units. A file for all macros handling shared units conversion remove CLOCKS_PER_MILLISEC from profile.h, include util/units.h instead add a cpp test for llapi indexes info * release index remove unused header from module-init * remove TODO * review fixes rename INFO_MODULES->info_redis rename MEMORY_HUMAN->MEMORY_MB (cherry picked from commit 175d787)

[MOD-8039] Reorganize stats files (#5329) * new file : info/INFO_MODULES responsible to expose all aggregated shard stats to INFO command move RS_moduleInfoFunc from module-init to move all add to info functions to a dedicated file: from global_stats.c FieldsGlobalStats_AddToInfo TotalGlobalStats_Queries_AddToInfo DialectsGlobalStats_AddToInfo IndexesGlobalStats_AddToInfo from config.c: RSConfig_AddToInfo introduce FieldsGlobalStats_GetIndexError to expose fields errors move queries stats from TotalGlobalStats to seperate struct QueriesGlobalStats * rediSearchAPI::RediSearch_TotalInfo calls IndexesInfo_TotalInfo from indexes_info IndexesInfo_TotalInfo (orignally rediSearchAPI::RediSearch_TotalInfo) calls IndexSpec_TotalMemUsage instead of rediSearchAPI::RediSearch_TotalMemUsage introduce indexes_info rename TotalSpecsFieldInfo and TotalSpecsInfo: specs->indexes move TotalIndexesFieldsInfo and TotalIndexesInfo from info_command to indexes_info move global stats to info dir move info_command to info dir introduce util/units. A file for all macros handling shared units conversion remove CLOCKS_PER_MILLISEC from profile.h, include util/units.h instead add a cpp test for llapi indexes info * release index remove unused header from module-init * remove TODO * review fixes rename INFO_MODULES->info_redis rename MEMORY_HUMAN->MEMORY_MB (cherry picked from commit 175d787) Co-authored-by: meiravgri <[email protected]>

* new file : info/INFO_MODULES responsible to expose all aggregated shard stats to INFO command move RS_moduleInfoFunc from module-init to move all add to info functions to a dedicated file: from global_stats.c FieldsGlobalStats_AddToInfo TotalGlobalStats_Queries_AddToInfo DialectsGlobalStats_AddToInfo IndexesGlobalStats_AddToInfo from config.c: RSConfig_AddToInfo introduce FieldsGlobalStats_GetIndexError to expose fields errors move queries stats from TotalGlobalStats to seperate struct QueriesGlobalStats * rediSearchAPI::RediSearch_TotalInfo calls IndexesInfo_TotalInfo from indexes_info IndexesInfo_TotalInfo (orignally rediSearchAPI::RediSearch_TotalInfo) calls IndexSpec_TotalMemUsage instead of rediSearchAPI::RediSearch_TotalMemUsage introduce indexes_info rename TotalSpecsFieldInfo and TotalSpecsInfo: specs->indexes move TotalIndexesFieldsInfo and TotalIndexesInfo from info_command to indexes_info move global stats to info dir move info_command to info dir introduce util/units. A file for all macros handling shared units conversion remove CLOCKS_PER_MILLISEC from profile.h, include util/units.h instead add a cpp test for llapi indexes info * release index remove unused header from module-init * remove TODO * review fixes rename INFO_MODULES->info_redis rename MEMORY_HUMAN->MEMORY_MB (cherry picked from commit 175d787)

* [MOD-8039] Reorganize stats files (#5329) * new file : info/INFO_MODULES responsible to expose all aggregated shard stats to INFO command move RS_moduleInfoFunc from module-init to move all add to info functions to a dedicated file: from global_stats.c FieldsGlobalStats_AddToInfo TotalGlobalStats_Queries_AddToInfo DialectsGlobalStats_AddToInfo IndexesGlobalStats_AddToInfo from config.c: RSConfig_AddToInfo introduce FieldsGlobalStats_GetIndexError to expose fields errors move queries stats from TotalGlobalStats to seperate struct QueriesGlobalStats * rediSearchAPI::RediSearch_TotalInfo calls IndexesInfo_TotalInfo from indexes_info IndexesInfo_TotalInfo (orignally rediSearchAPI::RediSearch_TotalInfo) calls IndexSpec_TotalMemUsage instead of rediSearchAPI::RediSearch_TotalMemUsage introduce indexes_info rename TotalSpecsFieldInfo and TotalSpecsInfo: specs->indexes move TotalIndexesFieldsInfo and TotalIndexesInfo from info_command to indexes_info move global stats to info dir move info_command to info dir introduce util/units. A file for all macros handling shared units conversion remove CLOCKS_PER_MILLISEC from profile.h, include util/units.h instead add a cpp test for llapi indexes info * release index remove unused header from module-init * remove TODO * review fixes rename INFO_MODULES->info_redis rename MEMORY_HUMAN->MEMORY_MB (cherry picked from commit 175d787) * fix includes in coord

meiravgri added 3 commits December 9, 2024 16:24

Merge remote-tracking branch 'origin/master' into meiravg_reorganize_…

81673ba

…INFO