[BUG/Discussion] Accessing undeclared keys in LUA scripts

[pior]

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:

asynq/internal/rdb/rdb.go

Lines 227 to 238 in 3dbda60

	var dequeueCmd = redis.NewScript(`
	if redis.call("EXISTS", KEYS[2]) == 0 then
	local id = redis.call("RPOPLPUSH", KEYS[1], KEYS[3])
	if id then
	local key = ARGV[2] .. id
	redis.call("HSET", key, "state", "active")
	redis.call("HDEL", key, "pending_since")
	redis.call("ZADD", KEYS[4], ARGV[1], id)
	return redis.call("HGET", key, "msg")
	end
	end
	return nil`)

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: #480 #724 #442

Discussion

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

[kamikazechaser]

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]

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).