[TODO]: Review Redis config

[polarathene]

Description

I saw these past comments of mine while looking into separate issue about our logging support, figured it might be worth raising a TODO issue if anyone has time.

For Debian Bookworm / DMS v14, I take it there is no issues with the Redis v7 upgrade, and no complaints about persistence config have come up since the review feedback.

Feel free to close this issue if it doesn't seem relevant, I'm not likely to have time to invest towards it myself. This is more for maintainer documentation / discovery reasons 😅

---

Presently dependent upon config shipped by default / Debian package, and modifying that:

docker-mailserver/target/scripts/startup/setup.d/security/rspamd.sh

Lines 117 to 145 in 23705e6

	# Sets up Redis. In case the user does not use a dedicated Redis instance, we
	# supply a configuration for our local Redis instance which is started later.
	function __rspamd__setup_redis() {
	if _env_var_expect_zero_or_one 'ENABLE_RSPAMD_REDIS' && [[ ${ENABLE_RSPAMD_REDIS} -eq 1 ]]; then
	__rspamd__log 'debug' 'Internal Redis is enabled, adding configuration'
	cat >"${RSPAMD_LOCAL_D}/redis.conf" << "EOF"
	# documentation: https://rspamd.com/doc/configuration/redis.html
	servers = "127.0.0.1:6379";
	expand_keys = true;
	EOF
	# Here we adjust the Redis default configuration that we supply to Redis when starting it.
	# NOTE: `/var/lib/redis/` is symlinked to `/var/mail-state/redis/` when DMS is started
	# with a volume mounted to `/var/mail-state/` for data persistence.
	sedfile -i -E \
	-e 's|^(bind).*|\1 127.0.0.1|g' \
	-e 's|^(daemonize).*|\1 no|g' \
	-e 's|^(port).*|\1 6379|g' \
	-e 's|^(loglevel).*|\1 warning|g' \
	-e 's|^(logfile).*|\1 ""|g' \
	-e 's|^(dir).*|\1 /var/lib/redis|g' \
	-e 's|^(dbfilename).*|\1 dms-dump.rdb|g' \
	/etc/redis/redis.conf
	else
	__rspamd__log 'debug' 'Rspamd will not use internal Redis (which has been disabled)'
	fi
	}

docker-mailserver/target/supervisor/conf.d/supervisor-app.conf

Lines 108 to 115 in 23705e6

	[program:rspamd-redis]
	startsecs=0
	stopwaitsecs=55
	autostart=false
	autorestart=true
	stdout_logfile=/var/log/supervisor/%(program_name)s.log
	stderr_logfile=/var/log/supervisor/%(program_name)s.log
	command=redis-server /etc/redis/redis.conf

Concerns / Advice provided previously:

• [BUG] rspamd forgets it learned spam and ham when docker-compose down and up is issued #3138 (comment)
• rspamd: rename ENABLE_REDIS & add persistence for Redis #3143 (review)

For maintenance, it'd probably be worthwhile to document the configuration changes. Some of the changes aren't obvious why they're done (as per review feedback below), where the only context available is git blame (until some eventual refactoring adds friction there 😬 )

---

References

Screenshots because I'm lazy.

Debian Bookworm upgrade (Redis v7)

The 2nd linked comment also expresses a concern for Redis v7 upgrade with Debian Bookworm:

Be mindful of Redis version and the major bump when Debian releases Bookworm, and reliance on implicit config (especially when it differs from what Redis ships / documents).

There was also another review comment there on the config contributed, with an intent for it to be reviewed for Debian Bookworm update (I don't think that was done yet?):

---

Persistence

Related was a concern about how persistence would be configured and changes in that support with v7 of Redis (additionally noting a difference in Debian shipped config vs upstream Redis):

Out of the two persistence links to Redis docs originally provided, only one is still valid. The other appears to now be located here.

---

Log config

Separate concern was expressed regarding log config (which may be more relevant [when Vector is adopted into DMS])#3561)):

From upstream Redis config docs (should be the default for us due to removing the config line, thus stdout to file through supervisord):

NOTE: /var/log/{redis,rspamd} exist as directories. Presumably left-over from Debian package installs? 🤷‍♂️

[georglauterbach]

I have just had a glance over the Rspamd persistence as I upgraded to the latest :edge and all seems to be working as expected.

[polarathene]

Part of the persistence concern was if RDB (default choice) was the right way to persist the data vs AOF or a mix (as the Redis docs explain). Since there's been no complaints thus far, perhaps that doesn't need to change but it'd be good to assess which is most appropriate before rspamd becomes the default.

[georglauterbach]

From what I understand, RDB is "the way to go for us", and this should hold with v7 as well, right? AOF does not seem to fit our needs at the moment.