docker-mailserver icon indicating copy to clipboard operation
docker-mailserver copied to clipboard

[TODO]: Review Redis config

Open polarathene opened this issue 1 year ago • 3 comments

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:

https://github.com/docker-mailserver/docker-mailserver/blob/23705e6712cd5b9cacf4cf8b5e7bfc3375e72e2e/target/scripts/startup/setup.d/security/rspamd.sh#L117-L145

https://github.com/docker-mailserver/docker-mailserver/blob/23705e6712cd5b9cacf4cf8b5e7bfc3375e72e2e/target/supervisor/conf.d/supervisor-app.conf#L108-L115

Concerns / Advice provided previously:

  • https://github.com/docker-mailserver/docker-mailserver/issues/3138#issuecomment-1453519599
  • https://github.com/docker-mailserver/docker-mailserver/pull/3143#pullrequestreview-1324908603

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).

image

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?):

image


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):

image

image

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])https://github.com/docker-mailserver/docker-mailserver/issues/3561)):

image

image

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

image

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

polarathene avatar Jan 30 '24 07:01 polarathene

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.

georglauterbach avatar Jan 30 '24 13:01 georglauterbach

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.

polarathene avatar Jan 30 '24 20:01 polarathene

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.

georglauterbach avatar Jan 31 '24 10:01 georglauterbach