SGrondin / bottleneck

Job scheduler and rate limiter, supports Clustering
MIT License
1.84k stars 79 forks source link

ERR SETTINGS_KEY_NOT_FOUND with redis 7.0.7 #219

Open SavvasGeethaRamu opened 1 year ago

SavvasGeethaRamu commented 1 year ago

ReplyError: ERR SETTINGS_KEY_NOT_FOUND at parseError (/var/app/current/node_modules/redis-parser/lib/parser.js:179:12) at parseType (/var/app/current/node_modules/redis-parser/lib/parser.js:302:14)

Do we have any workaround to resolve this issue? Or do we have any alternate library?

maselious commented 1 year ago

I have fixed this compatibility issue in the fork repo https://github.com/maselious/bottleneck

And published the package https://www.npmjs.com/package/@maselious/bottleneck

Feel free to use

mindrunner commented 11 months ago

This might fix the above mentioned error, but rate limiting does not work at all when using redis 7. Can someone confirm this?

mfcarey commented 11 months ago

@maselious thank you for sharing this update. As I'm comparing against the fork of Sergiiivzhenko, I noticed this diff. Could you help explain why that formatting change to the repo url was introduced? It may just be a syntactic convention I am unfamiliar with. thank you. (@mindrunner haven't had a chance to try this out yet but can aim to report back to your question upon doing so)

mindrunner commented 11 months ago

I tried different forks and concluded, this one is "stable" and working for me: https://github.com/usehaystack/bottleneck

I am doing a huge amount of requests and have many (~100) different bottlenecks with each one having its own config. Still in testing phase, but planning to roll this out to production. However, it's kinda scary to use an unmaintained library. Planning to reach out to Haystack and https://github.com/userashad in the hope, that we can get this stable and maintained to be safe for productive use.

maselious commented 11 months ago

@maselious thank you for sharing this update. As I'm comparing against the fork of Sergiiivzhenko, I noticed this diff. Could you help explain why that formatting change to the repo url was introduced? It may just be a syntactic convention I am unfamiliar with. thank you. (@mindrunner haven't had a chance to try this out yet but can aim to report back to your question upon doing so)

If shortly it is a best practice.

Longer (TLDR):

  1. The "git+" prefix makes it explicit that the URL is intended to be a Git repository URL. This helps tools and package managers like npm or yarn recognize the URL as a Git repository and use the appropriate methods to fetch and install the package. Without the "git+" prefix, the URL might be treated as a generic URL, which can lead to unexpected behavior.
  2. Depending on the project and package manager, there might be encountered different types of repositories (Git, Mercurial, Subversion, etc.). By specifying "git+", it is ensured that the URL is interpreted as a Git repository, regardless of the underlying version control system.
  3. When Git URLs used with the git prefix, package managers like npm or yarn know to use Git's cloning mechanisms to fetch the package. This allows you to clone the repository and work with it as if you had manually cloned it using Git.
mfcarey commented 11 months ago

thank you @maselious for the detailed explanation!

maselious commented 11 months ago

I encountered an issue when using Redis 7+ because they introduced a new "ERR" prefix for all errors. This change caused the error handlers in Bottleneck to malfunction. To address this, I discovered a fork of Bottleneck at https://github.com/Sergiiivzhenko/bottleneck, where the "UNKNOWN_CLIENT" issue had been resolved. However, the "SETTINGS_KEY_NOT_FOUND" issue remained unresolved in that fork.

In response, I decided to merge the commits from the fork and make additional modifications to support the "SETTINGS_KEY_NOT_FOUND" issue. Upon reviewing the code changes, I determined that there were no critical alterations that would affect the stability of the package.

I have been using my fork in a production environment with a Redis 7 cluster, and it has been working successfully. In the event that Bottleneck releases a critical patch or update, I will promptly incorporate those changes into my fork to ensure its continued reliability. However, i dont promise doing it immediately.

mindrunner commented 11 months ago

For some reason, your fork did not work for me with redis 7. I will definitely try again and see if I can reproduce the problems I have had yesterday. Do you see any major difference between your patches and the ones from over here? https://github.com/usehaystack/bottleneck

mindrunner commented 11 months ago

I am also experiencing this error here https://github.com/SGrondin/bottleneck/issues/195

Seems to happen rarely, but I am also not under heavy load situations yet.

theAlexPatin commented 10 months ago

Bumping this. Also experiencing this issue.

amansanghvi commented 9 months ago

Yep, I am also experiencing this - actually I resolved this. I saw that the API I was sending a request to was taking longer to respond that my expiry time, and so the request would go from executing, to scheduled (because the request expired and was retried) to done (because the previous request would return a response).