Skip to content

Commit dd30872

Browse files
authored
Preserve module load order in CONFIG REWRITE (valkey-io#3769)
## Why `CONFIG REWRITE` did not preserve module load order because the module registry was stored in a hash table with unspecified iteration order. This could write dependent modules before their dependencies, causing the next restart to fail. Runtime `MODULE LOAD` and `MODULE UNLOAD` operations could also change the resulting order. ## Fix Use a single ordered list as the module registry instead of maintaining a module dictionary and a separate load-order structure. - Register modules at the tail of the list. - Perform name lookups by scanning the list case-insensitively. - Remove modules directly from the list when unloaded. - Iterate the same list when writing `loadmodule` directives during `CONFIG REWRITE`. - Unload modules in reverse load order so dependents are unloaded before their dependencies. Module counts are normally small, so linear lookup keeps the implementation simple while preserving load order as a single source of truth. Reloading a module moves it to the tail, reflecting its new load time. ## Tests The module API tests verify that: 1. `CONFIG REWRITE` preserves module load order. 2. Static modules such as `lua` are not written to the configuration. 3. A rejected unload does not change the rewritten module order. 4. Unloading and reloading a module moves it to the tail. 5. The server can restart from the rewritten configuration. Fixes valkey-io#3119 --------- Signed-off-by: Taeknology <20297177+Taeknology@users.noreply.github.com>
1 parent f4dc3ca commit dd30872

5 files changed

Lines changed: 160 additions & 112 deletions

File tree

src/config.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1662,23 +1662,23 @@ static void rewriteConfigSocketBindOption(standardConfig *config, const char *na
16621662

16631663
/* Rewrite the loadmodule option. */
16641664
void rewriteConfigLoadmoduleOption(struct rewriteConfigState *state) {
1665-
if (dictSize(modules) == 0) {
1665+
if (listLength(modules) == 0) {
16661666
rewriteConfigMarkAsProcessed(state, "loadmodule");
16671667
return;
16681668
}
16691669

16701670
sds line;
1671+
listIter li;
1672+
listNode *ln;
16711673

1672-
dictIterator *di = dictGetIterator(modules);
1673-
dictEntry *de;
1674-
while ((de = dictNext(di)) != NULL) {
1675-
struct ValkeyModule *module = dictGetVal(de);
1674+
listRewind(modules, &li);
1675+
while ((ln = listNext(&li)) != NULL) {
1676+
struct ValkeyModule *module = listNodeValue(ln);
16761677
if (module->is_static_module) continue;
16771678
line = moduleLoadQueueEntryToLoadmoduleOptionStr(module, "loadmodule");
16781679
rewriteConfigRewriteLine(state, "loadmodule", line, 1);
16791680
}
1680-
dictReleaseIterator(di);
1681-
/* Mark "loadmodule" as processed in case modules is empty. */
1681+
/* Mark "loadmodule" as processed in case no modules are loaded. */
16821682
rewriteConfigMarkAsProcessed(state, "loadmodule");
16831683
}
16841684

0 commit comments

Comments
 (0)