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

PLAT-1059: Redis 7 is required for Enterprise #343

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jason-seqera
Copy link
Contributor

@jason-seqera jason-seqera commented Dec 10, 2024

Looks like this became required as far back as 23.3.

We're using 6.2 for 24.2+.

cc @llewellyn-sl

@jason-seqera jason-seqera self-assigned this Dec 10, 2024
Copy link

netlify bot commented Dec 10, 2024

Deploy Preview for seqera-docs ready!

Name Link
🔨 Latest commit 430d49e
🔍 Latest deploy log https://app.netlify.com/sites/seqera-docs/deploys/6758f46950a1cd00089136b5
😎 Deploy Preview https://deploy-preview-343--seqera-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@pditommaso
Copy link
Contributor

What's the reference for this change?

@jason-seqera
Copy link
Contributor Author

@pditommaso
Copy link
Contributor

It does not explain *why*

@swampie
Copy link
Member

swampie commented Dec 10, 2024

@jason-seqera @robnewman the rationale of this is because of Data Studio? Because 6.2 supports the command that Data studio uses. If there are customers on redis 6 this is a breaking change

@endre-seqera
Copy link
Member

endre-seqera commented Dec 10, 2024

Added some context and reasoning to the task: https://seqera.atlassian.net/browse/PLAT-1058?focusedCommentId=30478

With the change introduced on Aug 21, 2024 (JedisMemStoreServiceV2Impl) - we inadvertedly introduced a breaking change, as this implementation uses commands that need at least REDIS 6.2.0

So either we should reflect this minimum REDIS 6.2.0 requirement in the docs - or change the implementation and use only commands available on REDIS 6.0

@@ -132,7 +132,7 @@ Replace `{prefix}` in each configuration path with `/config/<application_name>`,
Configuration values that control Seqera's interaction with databases and Redis instances. `TOWER_DB_USER`, `TOWER_DB_PASSWORD`, and `TOWER_DB_URL` must be specified using environment variables during initial Seqera Enterprise deployment in a new environment. A new installation will fail if DB values are only defined in `tower.yml` or the AWS Parameter Store. Once the database has been created, these values can be added to `tower.yml` or [AWS Parameter Store](./aws_parameter_store.mdx) entries and removed from your environment variables.

:::note
From Seqera Enterprise version 23.3, Redis version 7 is officially supported. Follow your cloud provider specifications to upgrade your instance.
From Seqera Enterprise version 23.3, Redis version 7 is required. Follow your cloud provider specifications to upgrade your instance.
Copy link
Member

@endre-seqera endre-seqera Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @gwright99 correctly pointed out the change should be something like this:

From Seqera Enterprise version 23.3, Redis version 7 is officially supported. 
From Seqera Enterprise version 24.2, at least Redis version 6.2.0 is required. 
Follow your cloud provider specifications to upgrade your instance

(In case we decide to go with docs update, and not reverting the changes/changing the implementations)

@jason-seqera
Copy link
Contributor Author

So this seems TBD.

@jason-seqera
Copy link
Contributor Author

So 6.2 is required for 24.2.

Copy link
Contributor

@llewellyn-sl llewellyn-sl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding "or greater". Otherwise LGTM

@jason-seqera jason-seqera force-pushed the PLAT-1059-ds-redis-7-required branch from c6be391 to 430d49e Compare December 11, 2024 02:09
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.

5 participants