starred/asynq

Fork 0

mirror of https://github.com/hibiken/asynq.git synced 2026-04-25 23:15:51 +03:00

[GH-ISSUE #951] [BUG/Discussion] Accessing undeclared keys in LUA scripts #2485

New issue

Open

opened 2026-03-15 20:38:38 +03:00 by kerem · 2 comments

kerem commented

2026-03-15 20:38:38 +03:00

Owner

Originally created by @pior on GitHub (Oct 30, 2024).
Original GitHub issue: https://github.com/hibiken/asynq/issues/951

Originally assigned to: @hibiken on GitHub.

Describe the bug

I don't know whether this is a bug or not, but there may be an issue with the way this library uses LUA scripts:

Important: to ensure the correct execution of scripts, both in standalone and clustered deployments, all names of keys that a script accesses must be explicitly provided as input key arguments.
The script should only access keys whose names are given as input arguments.

Scripts should never access keys with programmatically-generated names or based on the contents of data structures stored in the database.

https://redis.io/docs/latest/commands/eval/

However, many Asynq LUA scripts are doing exactly that, mostly for the task key:
github.com/hibiken/asynq@3dbda60333/internal/rdb/rdb.go (L227-L238)

Stackoverflow says that specifying the keys is important when using Redis Cluster since it ensures that all keys are on the nodes where the lua script is running: https://stackoverflow.com/a/32091333
But this answer also mentions that the behavior is also undefined on a regular Redis 🤔.

There seems to be a few Github issues related to this: https://github.com/hibiken/asynq/issues/480 https://github.com/hibiken/asynq/issues/724 https://github.com/hibiken/asynq/issues/442

Discussion

I'm not sure whether there is a practical solution, but we should at least know and document this situation.

Originally created by @pior on GitHub (Oct 30, 2024). Original GitHub issue: https://github.com/hibiken/asynq/issues/951 Originally assigned to: @hibiken on GitHub. **Describe the bug** I don't know whether this is a bug or not, but there may be an issue with the way this library uses LUA scripts: > **Important**: to ensure the correct execution of scripts, both in standalone and clustered deployments, all names of keys that a script accesses must be explicitly provided as input key arguments. > The script should only access keys whose names are given as input arguments. > **Scripts should never access keys with programmatically-generated names or based on the contents of data structures stored in the database.** https://redis.io/docs/latest/commands/eval/ However, many Asynq LUA scripts are doing exactly that, mostly for the task key: https://github.com/hibiken/asynq/blob/3dbda603333da7c47449e3c1fc14f3c681ac58a3/internal/rdb/rdb.go#L227-L238 Stackoverflow says that specifying the keys is important when using Redis Cluster since it ensures that all keys are on the nodes where the lua script is running: https://stackoverflow.com/a/32091333 But this answer also mentions that the behavior is also undefined on a regular Redis 🤔. There seems to be a few Github issues related to this: https://github.com/hibiken/asynq/issues/480 https://github.com/hibiken/asynq/issues/724 https://github.com/hibiken/asynq/issues/442 **Discussion** I'm not sure whether there is a practical solution, but we should at least know and document this situation.

kerem added the

bug

investigate

labels

2026-03-15 20:38:38 +03:00

kerem commented

2026-03-15 20:38:49 +03:00

Author

Owner

@kamikazechaser commented on GitHub (Oct 31, 2024):

Definitely an anti-pattern against redis recommendations. But in most cases these are dynamic keys being passed, not sure how we can go around these. I'll read up more on this.

@kamikazechaser commented on GitHub (Oct 31, 2024): Definitely an anti-pattern against redis recommendations. But in most cases these are dynamic keys being passed, not sure how we can go around these. I'll read up more on this.

kerem commented

2026-03-15 20:38:54 +03:00

Author

Owner

@kamikazechaser commented on GitHub (Oct 31, 2024):

I think we should remove the line on the README and add a caveat about redis cluster compatibility until we confirm ALL scripts are redis cluster compatible (which isn't the case now).

@kamikazechaser commented on GitHub (Oct 31, 2024): I think we should remove the line on the README and add a caveat about redis cluster compatibility until we confirm ALL scripts are redis cluster compatible (which isn't the case now).

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

starred/asynq#2485

No description provided.

Rows
Columns