rspamd: rename ENABLE_REDIS & add persistence for Redis

[georglauterbach]

Description

This PR

1. renames ENABLE_REDIS -> ENABLE_RSPAMD_REDIS, see 0649683 for details
2. enables persistence for the Redis DB as detailed in [BUG] rspamd forgets it learned spam and ham when docker-compose down and up is issued #3138

Fixes #3138

Type of change

• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation (README.md or the documentation under docs/)
• If necessary I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[polarathene]

LGTM 👍

• 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).
• Persistence - This PR enables (RDB type) implicitly (and whatever else that upstream Redis defaults do not).
• Logging - Discussed syslog instead of supervisor managed, but I wouldn't worry about it for now.
• Docs - Possible improvements, up to you. This review is self-documenting enough for me atm 😅

---

Bullseye is shipping v6 of Redis, note that the upcoming Bookworm release later this year bumps that to v7 (released April 2022).

The change from a command to using the default shipped config, was primarily motivated by the implicit config to enable persistence. Note that this is not advised by upstream:

Redis is able to start without a configuration file using a built-in default configuration, however this setup is only recommended for testing and development purposes.

The proper way to configure Redis is by providing a Redis configuration file, usually called redis.conf.

I believe that is referring to just running redis-server and using args to override the defaults (which presumably match the upstream default config, which Debian isn't aligned with).

Just something to keep in mind, especially once the transition to v7 arrives and implicit config defaults are relied on.

Persistence

Persistence support has some behaviour differences between these two versions, but most seem to be related to AOF type which does seem nice at least for v7, but RDB type disadvantages don't appear to be a concern for our usage, and should be simpler to backup (potentially not as much for some users, where Docker is wrapped in a VM such as on macOS, which IIRC uses file sync to manage volume mounts).

Default config is using RDB (Present in Debian shipped config, upstream Redis doesn't):

# Enable RDB
save 900 1
save 300 10
save 60 10000

# Disable AOF
appendonly no

Translates to persist triggers:

• After 900 sec (15 min) if at least 1 key changed
• After 300 sec (5 min) if at least 10 keys changed
• After 60 sec if at least 10000 keys changed

Also, as mentioned in the associated issue, when persistence is performing an update to disk, double the memory used for the in-memory store is required as a precaution (but unlikely needed due to CoW).

It might be worth referencing that Redis FAQ entry in our docs for Redis, or just expecting a maintainer or user to know about it when troubleshooting. The link explains how to avoid the issue, but I think is required to be done on the host or via adding a capability to the compose config.

For now, it's at least documented in this review and associated issue.

---

Logging

This section can probably can be ignored for this PR.

I'm not familiar with how to configure syslog to redirect logs to a specific file for a given syslog tag, and not too interested in maintaining that. May be better to wait until Vector is considered.

If we want to instead log to syslog instead of having supervisord write the /dev/stdout / /dev/stderr logs to a file it manages, we can use syslog-enabled yes. Here's relevant redis config:

# To enable logging to the system logger, just set 'syslog-enabled' to yes,
# and optionally update the other syslog parameters to suit your needs.
# syslog-enabled no

# Specify the syslog identity.
# syslog-ident redis

# Specify the syslog facility. Must be USER or between LOCAL0-LOCAL7.
# syslog-facility local0

Here are the Syslog facility choices that can be used. syslog-facility default is probably fine, as is syslog-ident.

Looking at the current /etc/rsyslog.conf, this would route logs to /var/log/syslog for that generic facility, and probably also to /var/log/messages (intended for info, notice, warn logs, but excludes other facilities like mail).

[polarathene]

These changes seem fine. Some aren't required but are good to retain if base image were to change in future, since we shouldn't rely on implicit config where it matters 👍

loglevel is only valid question below.

---

Debian patches the upstream config in their package.

• 👍 bind dropping IPv6 localhost? Debian (or v6 of Redis) expects to successfully bind, v7 Redis config uses 127.0.0.1 -::1 to optionally bind IPv6 but not fail to start if unsuccessful. May not be compatible with v6, no apparent need for binding to IPv6 👍
• 👍 daemonize - Reverted to upstream Redis default.
• ❓ port - In both configs this is already 6379.
• ❓ loglevel - Any particular reason for adjusting to warning instead of the default notice?
• 👍 logfile - Reverted to upstream Redis default (Debian uses /var/log/redis/redis-server.log).
• ❓ dir - Debian already adjusts this to /var/lib/redis (from upstream default of ./)
• 👍 dbfilename - A custom name seems like a good idea 👍 I'd suggest dms-rspamd-dump.rdb, but perhaps Redis is useful for other services (PostSRSd v2 can leverage it), so keeping this service agnostic is less friction to support 👍

[georglauterbach]

Thanks for the detailed comment @polarathene! I will make sure to check this out once we update the base image.

[github-actions]

Documentation preview for this PR is ready! 🎉

Built with commit: 306e5ba