[BUG] Identified a bug in rewrite loadmodule

[vikram-krishna-s]

Hi @soloestoy!

I maintain a secondary.confconfiguration file for loadmodule and other configurations and include them it in the primary.conf file which I use to start the redis-server (Redis version=7.0.10).

The primary.conf and secondary.conf look like this,

primary.conf

...
include /home/vikram/redis7/secondary.conf
...

secondary.conf

...
loadmodule /home/vikram/redis7/aclcheck.so
...

1. I did not encounter any errors when I first started the redis-server withprimary.conf.

redis7>./redis-server ./primary.conf

10755:C 04 Apr 2023 15:16:09.690 # oO0OoO0OoO0Oo Redis is starting oO0OoO0OoO0Oo
10755:C 04 Apr 2023 15:16:09.690 # Redis version=7.0.10, bits=64, commit=00000000, modified=0, pid=10755, just started
10755:C 04 Apr 2023 15:16:09.690 # Configuration loaded
10755:M 04 Apr 2023 15:16:09.692 * monotonic clock: POSIX clock_gettime
10755:M 04 Apr 2023 15:16:09.693 # Server initialized
10755:M 04 Apr 2023 15:16:09.694 * Module 'aclcheck' loaded from /home/vikram/redis7/aclcheck.so
10755:M 04 Apr 2023 15:16:09.695 * Ready to accept connections

2. Then after executing CONFIG REWRITE , the primary.conf file was appended with 'loadmodule /home/vikram/redis7/aclcheck.so' as said in Modules: rewrite loadmodule and extend LIST reply #4848.

redis7>./redis-cli CONFIG REWRITE

primary.conf file after CONFIG REWRITE

include /home/vikram/redis7/secondary.conf

# Generated by CONFIG REWRITE
...
loadmodule /home/vikram/redis7/aclcheck.so
...

3. Now on redis-server restart, I am facing the below error because during startup there are two loadmodule directives for the same module, one from secondary.conf and one from primary.conf.

redis7> ./redis-server ./primary.conf

11671:C 04 Apr 2023 15:17:37.469 # oO0OoO0OoO0Oo Redis is starting oO0OoO0OoO0Oo
11671:C 04 Apr 2023 15:17:37.469 # Redis version=7.0.10, bits=64, commit=00000000, modified=0, pid=11671, just started
11671:C 04 Apr 2023 15:17:37.469 # Configuration loaded
11671:M 04 Apr 2023 15:17:37.470 * monotonic clock: POSIX clock_gettime
11671:M 04 Apr 2023 15:17:37.472 # Server initialized
11671:M 04 Apr 2023 15:17:37.473 * Module 'aclcheck' loaded from /home/vikram/redis7/aclcheck.so
11671:M 04 Apr 2023 15:17:37.473 # Module /home/vikram/redis7/aclcheck.so initialization failed. Module not loaded
11671:M 04 Apr 2023 15:17:37.473 # Can't load module from /home/vikram/redis7/aclcheck.so: server aborting

Originally posted by @vikram-krishna-s in #4848 (comment)

[soloestoy]

interesting, we didn't consider the include file when CONFIG REWRITE, so the configs in include may be wrong after rewrite, and the loadmodule is the very special one.

Seems we need exclude configs in include file when rewriting, thinking.

[enjoy-binbin]

a link that I feel is related to recently #11736

[oranagra]

Seems we need exclude configs in include file when rewriting, thinking.

@soloestoy can you elaborate?

just keep a flag for each config that was loaded from included file, and avoid writing it to the main one on a rewrite?
note that configs can be repeated (exist in both, one overrides the other), and some directives like modules and acl, can be repeated without overriding each other.
so this trivial approach will solve some cases, where it's a clear cut where the configs are, but not others.

i imagine that properly solving the problem and allowing these two features to be mixed is too complicated and we should just either disallow that, or advise against it.
but i'm obviously open for suggestions..