Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for Redis Sentinel #375

Merged

Conversation

DiscowZombie
Copy link
Contributor

@DiscowZombie DiscowZombie commented Dec 31, 2023

Current limitations:

  • The node password and the sentinel password should be the same (if any) ;
  • IPv6 is not supported because the split(":") assume only one : (between the IP and the port), which isn't true for IPv6.

Limitations gone since 39bf095

Testing setup

config.yml

redis:
  # When set to true, the redis service will be enabled
  enabled: true
  # The host of your redis server
  host: '127.0.0.1:26379,127.0.0.1:26380,127.0.0.1:26381'
  # The port of your redis server
  port: 6379
  # The password to your redis server.
  # Leave set to "none" to connect without authentication
  password: 'passwd'
  # If SSL should be used to connect to your redis server (only if you have setup TLS)
  useSSL: false
  # The channel to sync with. When configured, OpenAudioMc will only sync to servers on redis with the same section
  section: event
  # When using Redis Sentinel, you should set the master set to use. Leaves empty to use a single server Redis cluster
  sentinel-master-set: 'master'

docker-compose.yml

version: '3.9'

services:
  master:
    image: "bitnami/redis:7.2.1"
    environment:
      - REDIS_DISABLE_COMMANDS=FLUSHDB,FLUSHALL,CONFIG
      - REDIS_AOF_ENABLED=no
      - ALLOW_EMPTY_PASSWORD=no
      - REDIS_PASSWORD=passwd
      - REDIS_REPLICATION_MODE=master
    network_mode: host
  replica:
    image: "bitnami/redis:7.2.1"
    environment:
      - REDIS_REPLICATION_MODE=slave
      - REDIS_MASTER_HOST=127.0.0.1
      - REDIS_MASTER_PASSWORD=passwd
      - REDIS_PASSWORD=passwd
      - REDIS_MASTER_PORT_NUMBER=6379
      - REDIS_PORT_NUMBER=6380
    depends_on:
      - master
    network_mode: host
  sentinel-1:
    image: 'bitnami/redis-sentinel:7.2.1'
    environment:
      - REDIS_MASTER_HOST=127.0.0.1
      - REDIS_MASTER_SET=master
      - REDIS_MASTER_PASSWORD=passwd
      - REDIS_SENTINEL_PASSWORD=passwd
      - REDIS_SENTINEL_DOWN_AFTER_MILLISECONDS=30000
      - REDIS_SENTINEL_PORT_NUMBER=26379
    depends_on:
      - master
      - replica
    network_mode: host
  sentinel-2:
    image: 'bitnami/redis-sentinel:7.2.1'
    environment:
      - REDIS_MASTER_HOST=127.0.0.1
      - REDIS_MASTER_SET=master
      - REDIS_MASTER_PASSWORD=passwd
      - REDIS_SENTINEL_PASSWORD=passwd
      - REDIS_SENTINEL_DOWN_AFTER_MILLISECONDS=30000
      - REDIS_SENTINEL_PORT_NUMBER=26380
    depends_on:
      - master
      - replica
    network_mode: host
  sentinel-3:
    image: 'bitnami/redis-sentinel:7.2.1'
    environment:
      - REDIS_MASTER_HOST=127.0.0.1
      - REDIS_MASTER_SET=master
      - REDIS_MASTER_PASSWORD=passwd
      - REDIS_SENTINEL_PASSWORD=passwd
      - REDIS_SENTINEL_DOWN_AFTER_MILLISECONDS=30000
      - REDIS_SENTINEL_PORT_NUMBER=26381
    depends_on:
      - master
      - replica
    network_mode: host

@Mindgamesnl
Copy link
Owner

Mindgamesnl commented Jan 1, 2024

Looks good! few chores that'll have to be done before this can go into prod (some of which I can pickup myself after Jan 4th)

  • Setup a migration for the config
  • Implement in vista's redis configuration (server)
  • Merge the docker file you provide (thanks for that by the way!) into dev-resources as a test target

@DiscowZombie DiscowZombie marked this pull request as ready for review January 7, 2024 10:58
@DiscowZombie
Copy link
Contributor Author

Hello @Mindgamesnl!

I completed my tests with vistas, and everything seems to work fine! (But fell free to test on our own too!)
I was forced to duplicate the RedisUtils because the class is relocated on the plugin module, but not on the vistas one. If you got any idea to fix this, I would be happy!

@Mindgamesnl
Copy link
Owner

Thanks a ton, looks great!

And yeah, the duplication of RedisUtils is unfortunate but a requirement unless we were to also remap the redis for vistas, but there's no real point into doing so there

I'll merge it in the development branch, do some local testing and get it included in the next release.

Thank you so much!

@Mindgamesnl Mindgamesnl changed the base branch from master to development January 9, 2024 11:47
@Mindgamesnl Mindgamesnl merged commit 86260ff into Mindgamesnl:development Jan 9, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants