-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
fix: Redis driver throws TypeError under high load #7959
Conversation
Fixes "TypeError: result is not iterable at RedisQueueDriverConnection.retrieveForProcessing". The return value of `execAsync` is undefined if the redis transaction fails due to out of memory errors, see https://redis.io/docs/interact/transactions/#errors-inside-a-transaction. This is a regression of 101b85f.
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 8 Ignored Deployments
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7959 +/- ##
==========================================
+ Coverage 48.02% 48.04% +0.02%
==========================================
Files 154 154
Lines 21019 21020 +1
Branches 5264 5264
==========================================
+ Hits 10095 10100 +5
+ Misses 10723 10719 -4
Partials 201 201
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Hi @schneefux 👋 I'll let @paveltiunov to review this one but, just FYI, Redis support is deprecated and is subject to be removed from Cube: https://cube.dev/docs/product/deployment#redis |
@schneefux Redis support is deprecated, and we do not accept any changes to this code base. Please consider moving to Cube Store. |
Since migrating to cubestore in early fall last year, I've had severe performance issues, the cube API servers and cubestore would randomly hang up. This was probably caused by a memory leak introduced in 0.34, see #7545. I have been running the Redis store for multiple years without any issues so I'd prefer to use it as long as possible until cubestore is more stable. |
@schneefux Sorry to hear that! We didn't see anything like that in our production environments, so it might be configuration differences. We're open to merging any Cube Store fixes regarding OOM, and I believe this is the only way we have right now. |
Check List
Issue Reference this PR resolves
N/A
Description of Changes Made (if issue reference is not provided)
Fixes "TypeError: result is not iterable at RedisQueueDriverConnection.retrieveForProcessing". The return value of
execAsync
is undefined if the redis transaction fails due to out of memory errors, see https://redis.io/docs/interact/transactions/#errors-inside-a-transaction. This is a regression of 101b85f.