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

Add support for static topology configuration for the replication client #444

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

Ladicek
Copy link
Contributor

@Ladicek Ladicek commented Apr 5, 2024

Includes other changes, especially:

  • introduced dedicated RedisReplicationConnectOptions (internal-only, but important anyway)
  • fixed sending the READONLY command to replicas (only in the cluster mode, not in replication mode)
  • documentation improvements
  • testing improvements

Fixes #343

The `READONLY` command is sent to replicas as a setup command before
the connection is handed to the caller. This command should only be
sent to replicas in the cluster mode (see [1]). The sentinel mode
didn't use it, but the replication mode did, which is a bug and
causes all replicas to be ignored (because the setup command fails).
This commit fixes it.

[1] https://redis.io/commands/readonly/
- update Testcontainers
- add logging configuration, more readable than JUL default
For each Redis deployment style (standalone, replication, sentinel, cluster),
there's a class that can be used as a JUnit test rule and internally uses
Testcontainers to run Redis. They provide some basic startup guarantees,
so users don't need to wait for a server to come online etc.

The helper class for standalone Redis allows customizing the Redis version,
setting a password, and enabling TLS (including mutual TLS). The multi-node
variants assume basic configuration and don't allow customization.

These helper classes use the Bitnami images, because they provide environment
variables for all the necessary customizations. The multi-node variants also
use Docker Compose.
@Ladicek Ladicek changed the title Add support for static topology configuration for the replication client @Ladicek Add support for static topology configuration for the replication client Apr 5, 2024
The improvements mostly consist of using the new Redis helper classes
for running standalone/replication/sentinel/cluster Redis, instead
of the previous approach with a single container for multi-node
deployments, which is not realistic and can hide bugs, but there are
other improvements too. Notably, the TLS test uses actual Redis TLS
support instead of a Vert.x-based TLS proxy, and the multi-node tests
verify that data replication actually works. Some useless tests are
deleted, too.
@Ladicek
Copy link
Contributor Author

Ladicek commented Apr 5, 2024

The interesting thing about the testing improvements here is support for non-Linux OSs. I don't know if it works or not, I'd appreciate if anyone with Windows or macOS tested this (just mvn clean verify) and reported back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Reading from read replicas when using Elasticache (cluster mode disabled)
1 participant