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

Using a redis instance with enableReadyCheck or maxRetriesPerRequest is not permitted. #2186

Closed
OliverwengFiltered opened this issue Oct 27, 2021 · 17 comments
Labels

Comments

@OliverwengFiltered
Copy link

getting this error after upgrading to latest v4.

i'm using ioredis.

any insights?

@manast
Copy link
Member

manast commented Oct 28, 2021

This is because you are probably reusing queues and instantiating your own redis instances, if so you need to pass to these instances at least these options:

new Redis({
  maxRetriesPerRequest: null,
  enableReadyCheck: false
 });

@manast
Copy link
Member

manast commented Oct 28, 2021

https://github.com/OptimalBits/bull/blob/develop/CHANGELOG.md#400-2021-10-27

@KishanKumarNR
Copy link

I am alredy using these 2 flags in the config
maxRetriesPerRequest: null,
enableReadyCheck: false

but after upgrade to I also get an same error

Init server failed: Using a redis instance with enableReadyCheck or maxRetriesPerRequest is not permitted.

@manast
Copy link
Member

manast commented Nov 12, 2021

This is the code that generates that error:

      if (
        client.options.enableReadyCheck ||
        client.options.maxRetriesPerRequest
      ) {
        throw new Error(errors.Messages.MISSING_REDIS_OPTS);
      }

jujaga added a commit to jujaga/common-hosted-email-service that referenced this issue Dec 10, 2021
- OptimalBits/bull#2186

Signed-off-by: Jeremy Ho <jujaga@gmail.com>
@jujaga
Copy link

jujaga commented Dec 10, 2021

We have a manually defined function defined for createClient as we need to ensure we are getting either standalone or cluster connections based on our environment configuration. Those two parameters are set as well for the redisOptions so that ioredis knows to instantiate with those settings.

We've observed that it seems to behave if we connect to a standalone instance of Redis, but it is now failing to do so with a manually defined Redis.Cluster instantiation, even though we are definitely passing those new parameters into the cluster definition as well. We had to rollback to a pre-4.x version because of this issue.

Ref our createClient implementation here: https://github.com/bcgov/common-hosted-email-service/blob/571836c2a76a6e40587664336e7b6a7be49095c3/app/src/services/queueConn.js#L28

@tjhelsel
Copy link

I'm having this issue as well when trying to run it with a Redis Cluster.

The problem is that the check that the ClusterOptions shape is different from what is checked in the code that @manast linked:

This is the code that generates that error:

      if (
        client.options.enableReadyCheck ||
        client.options.maxRetriesPerRequest
      ) {
        throw new Error(errors.Messages.MISSING_REDIS_OPTS);
      }

The client for clusters looks like the below; maxRetriesPerRequest and enableReadyCheck are on the nested redisOptions property, rather than on options itself:

{
  options: {
    redisOptions: { maxRetriesPerRequest: null, enableReadyCheck: false }
}

It looks like reverting to an older version is the best option at the moment, but wondering if a fix is in the works?

@manast manast closed this as completed in 071c51d Dec 14, 2021
github-actions bot pushed a commit that referenced this issue Dec 14, 2021
## [4.1.4](v4.1.3...v4.1.4) (2021-12-14)

### Bug Fixes

* **queue:** check redisOptions is available fixes [#2186](#2186) ([071c51d](071c51d))
@manast
Copy link
Member

manast commented Dec 14, 2021

🎉 This issue has been resolved in version 4.1.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

@OliverwengFiltered
Copy link
Author

Hi @manast

do you mind elaborate a bit on what's changes in v4.1.4, it looks like when I initialize my redis instance I still have to put the below two props in place, otherwise it throws the same error.

        return new Redis(process.env.REDIS_URL, {
            maxRetriesPerRequest: null,
            enableReadyCheck: false
        });

@manast
Copy link
Member

manast commented Dec 21, 2021

@OliverwengFiltered can you provide a test? because in our unit tests we do not specify these options and the tests are passing.

@OliverwengFiltered
Copy link
Author

@manast
I put together a very simple demonstration to indicate that the two props seem still needed.

https://github.com/OliverwengFiltered/bull-redis-demo

just clone the above repo and npm install && node worker.js

you will see the error Using a redis instance with enableReadyCheck or maxRetriesPerRequest is not permitted. still showing up.

@tjhelsel
Copy link

tjhelsel commented Jan 8, 2022

Hi @manast

do you mind elaborate a bit on what's changes in v4.1.4, it looks like when I initialize my redis instance I still have to put the below two props in place, otherwise it throws the same error.

        return new Redis(process.env.REDIS_URL, {
            maxRetriesPerRequest: null,
            enableReadyCheck: false
        });

These properties must still be set to a falsy value; v4.1.4 did not change that. The issue that v4.1.4 fixed is how this check is handled for Redis clusters. Previously, setting the above values to null and false would not avoid the error for Redis clusters; but with v4.1.4, the error is not thrown. (This has to do with the shape of the cluster options config, per my previous post.)

@manast
Copy link
Member

manast commented Jan 10, 2022

@OliverwengFiltered as explained above, if you are instantiating your own Redis instances you need to pass the maxRetriesPerRequest and enableReadyCheck options, you are not doing that here: https://github.com/OliverwengFiltered/bull-redis-demo/blob/main/redis.js

The reason for this requirement is that these settings are necessary for ioredis to actually keep the connections working in all cases even when servers go down, etc. Otherwise, people report these issues as bugs in Bull which is quite time-consuming.

@OliverwengFiltered
Copy link
Author

@manast
I see what you are saying, thanks for the explanation.

@koresar
Copy link

koresar commented Jan 31, 2022

Nice fix Manuel.
I almost reported the bug myself. :) Thanks for this warning in the console. Otherwise, I would have never found this little fact that IORedis disconnects!

@detunjiSamuel
Copy link

detunjiSamuel commented Mar 3, 2022

@OliverwengFiltered as explained above, if you are instantiating your own Redis instances you need to pass the maxRetriesPerRequest and enableReadyCheck options, you are not doing that here: https://github.com/OliverwengFiltered/bull-redis-demo/blob/main/redis.js

The reason for this requirement is that these settings are necessary for ioredis to actually keep the connections working in all cases even when servers go down, etc. Otherwise, people report these issues as bugs in Bull which is quite time-consuming.

i tried this but it still gives the same error .

@abustya
Copy link

abustya commented Jan 16, 2023

if you are instantiating your own Redis instances you need to pass the maxRetriesPerRequest and enableReadyCheck options

@manast I think this should be documented, somewhere here: https://docs.bullmq.io/guide/connections
Also, the provided example for "Reuse the ioredis instance" is missing these options.

@manast
Copy link
Member

manast commented Jan 16, 2023

@abustya you are currently getting an error/warning if using the settings incorrectly, so it should be clear enough I think 🤔

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

No branches or pull requests

8 participants