It has recently come to my attention a text in both the new FUNCTION command as well as in the older EVAL (from https://redis.io/commands/eval) command that states this

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.

I am pretty sure this has been a recommendation before and not a "should never", specially in a standalone scenario or if using "hash tags" in a clustered environment (https://redis.io/topics/cluster-tutorial).

Now, we have been generating keys inside lua scripts for a decade in Bull (https://github.com/optimalbits/bull) and BullMQ(https://github.com/taskforcesh/bullmq) without any issues. Thousands of companies depends on these libraries to work reliable every day so this change in the text has worried me. I suspect this has to do with the most strict rules you are enforcing on the new FUNCTION since the documentation changes comes from this PR: https://github.com/redis/redis-doc/pull/1725

So please, could you clarify this change in the requirements of EVAL and if this is going to break future deployments of Bull/BullMQ as new versions of Redis are released?

Comment From: oranagra

IIRC We didn't change anything or plan to change anything soon, but there are these potential issues: 1. the client is generating a different script on every execution, embedding the key names or other variables in the script code, it could cause the script cache to consume a lot of memory and explode. 2. referring to key names that are not mentioned as argument can cause cluster aware clients not to route the script to the right cluster node, or cause redis itself not to reject it with MOVED error. 3. in cluster mode, referring to other key names inside the script could cause some commands in the script to fail with Script attempted to access a non local key in a cluster node, after some commands already succeeded (breaking atomicity). 4. same as the above with ACL, if the script starts executing performs some modifications and then one command inside the script fails on ACL check (see scriptVerifyACL)

The only thing here that recently changed is the ACL check, in which the improved ACL in 7.0 can allow some commands access to some keys, and other commands access to other keys.

@itamarhaber @madolson anything i'm missing?

Comment From: manast

@oranagra thanks for the answer.

Regarding point 2 and 3, AFAIK using "hash tags", i.e. all the keys will use the same slot should avoid this issue, at least this is how users using redis clusters manage to use Bull/BullMQ successfully. Point 4 should be OK, if a script fails due to this then the user can always fix the ACL rights.

Point 1 on the other hand is leaving me confused. How do you mean the client is generating a different script? it is using EVALSHA as normally, it is just that the script is generating new keys inside the script itself 🤔

Comment From: oranagra

for [1] i meant a case that a client uses EVAL and embedding a different key name or a different variable as part of the script. like this Python code:

def something(r, some_valriable):
    r.eval("redis.call('set', 'x', '%s')" % some_valriable)

i.e. regardless of being a key name or not, each time you call this method with a different value it generates a new script in the cache.

Comment From: itamarhaber

The new wording is mine. It is there to serve as "here be dragons" for those who actually read the docs, as [1] is an anti-pattern that we encounter all-too-frequetly. That said, Oran had captured everything and there are no official plans to worsen the existing afaik. Put differently, as long as a) you use KEYS[] and/or hashtag everything, CROSSSLOT ain't a concern and should be future-proof.

Comment From: dbrgn

Thanks for the replies and clarifications, I tried to summarize it in this SO answer: https://stackoverflow.com/a/75192875/284318

If anything in there is not correct, I'm happy about corrections!