Good work! ping @redis/core-team to review.

About scripts persisting, I prefer NO, since we never guarantee scripts could be persisted, and as antirez said, it can make replication clearer.

And I think we can allow eviction in lua now, fix issue #8478.

@zhugezy thank you and welcome.
@soloestoy i was under the impression we agreed this task is assigned to @yoav-steinberg .
in any case, i think this was done too early, and will need to be re-done once #9780 is merged.
In the meanwhile, we can start discussing the details here (instead of the other 3 existing open issues / PRs on the same subject).
There are currently arguments for both keeping scripts deterministic (i'll let @yossigo post them), as well as keeping the persistence (which you argued for).

Regarding the persistence, the EVAL design was very clear from the get go that clients can't assume the script is cached, and the EVALSHA is an optimization that the client should be prepared for any case the script is not cached.
I know users kept arguing that it's buggy, without reading the theory and design behind it (which is that the script is part of the client application, that's executed in the server, and not at all the responsibility of the server, which is why it was not named and not versioned)

In redis 4.0, because of PSYNC2, these cases where the script is missing got rarer, so users may have got accustomed to it.
and now that we remove EVAL propagation, in theory we can go back on that, and never replicate or persist the EVAL scripts.
This will make a the EVAL scripts design and the differences from Redis Functions clearer, and force users to realize that.
but indeed some users will see it as a step backwards.

@soloestoy i was under the impression we agreed this task is assigned to @yoav-steinberg .

Oh I didn't notice it, maybe you mean this approach #8478 (comment)? But I think after this PR we can allow eviction in lua just like MULTI/EXEC, so the problem can be fixed smoothly and no need break atomic.

I thought we agreed that both of these will be assigned to Yoav, but maybe don't recall something.
Anyway, it's too early to code it, but not too early to discuss and decide...

Regarding the OOM, even if we allow eviction, the problem still exists since the server might be configured not to allow eviction. And also, there's the other discussion to make about deterministic execution (which I mentioned above but left for Yossi to present), which may lead to concluding that we still can't evict inside a script.
But anyway, let's not mix these two topics, and discuss each of them in a separate PR.

Let's first debate about propagation and persistence.
I remember Salvatore vocally mentioning that users should not assume scripts are cached, possibly even if the same connection that loaded them is still alive.
I don't remember where it was and what was the argument, but the thing is that the design is that the script and its exact content are actually part of the caller app (not the server's responsibility), and it should always be prepared to reload it.
The main feature is EVAL, and the SHA part is just an optimization.

Persistence and replication: the actual guarantee as documented was never very clear and at some point (#5292 (comment)) it was limited to the scope of a single connection (although SCRIPT FLUSH can break that as well).

Redis 7 and Redis Functions is a good opportunity to clean this up and re-set the client expectations, so I think it makes sense to completely drop script replication.

Eviction: I'm not in favor of that. While it's not strictly needed for consistency any longer, I think guaranteeing atomicity is still good practice, especially given that scripts are not designed to be long running. Having scripts suddenly see keys vanishing keys during execution in Redis 7 seems like a problem to me.

Deterministic execution: This too is not strictly needed but I think not being able to enforce it, at least optionally, is a potential regression. I think it's reasonable to expect identical results if you run the same script on multiple instances with the same dataset (e.g. running tests scenarios on different envs, prod vs. staging/dev, etc.).

Stage summary: looks like we have reached a consensus that script persistence/replication can & should be dropped, and haven't seen any objections in the past few days. If it's done, then we can move onto the next topic.

For deterministic execution, I think non-deterministic results are consistent with users' expectations when they put "random" commands in scripts/functions and send them to multiple instances, while deterministic may not. So why bother making the result deterministic? Anyway, it would be better if someone could cite their examples about it.

@zhugezy we had a discussion about this these topics in a core-team meeting today.
At first, we concluded that we better keep the feature in redis that allows scripts to be deterministic, but maybe let the user control if it's enabled or disabled.
we considered to either:

1. if we intend to keep this feature forever, then have it disabled in functions but default, and keep it enabled in eval by default (so we don't break existing scripts).
2. or if we intend delete it some day, we can start by making it disabled by default for both functions and eval (so users can enable it if the run into trouble), and then consider deleting the code at a later stage (kinda how the lua_replicate_commands feature was introduced in baby steps too).

but then we realized that since lua_replicate_commands was changed to default of true, it also meant that scripts where allowed to be non-deterministic by default, and we are not aware of anyone who complained.
so in fact we're already half way through on our way to delete that feature, and we can take the next step now (actually delete it).

regarding script persistence and replication, as was said before, it was never meant to be persisted and replicated, and even if some users started getting accustomed to it since PSYNC2, it's actually likely that it'll cause them bugs, since the scripts are sometimes replicated, but they're never persisted. we wanna proceed and delete that feature completely.

so i think the next step is to wait for the functions PR to be merged, and then re-do this deletion work.

@oranagra Totally agreed.
BTW should I set this PR as a draft, or close it now and make a new PR after Function merged?

i think you can just force-push the new version into it when it's ready.
no sense in opening another thread when we have the discussion here.

btw, i see this PR doesn't remove the MASTER and SLAVE consistency with EVALSHA replication test, i guess it should.
this test keeps hanging every day in the freebsd CI (i guess for being slow).
not sure what was the process to decide which tests to delete, was it just ones who failed?

@oranagra Oh, I didn't notice this test... I just checked all tests in scripting.tcl, tests with keywords like "lua-replicate-commands","redis.replicate_commands" or else and other failed tests, which I thought are all of them. I'll check other test files later.

@zhugezy we discussed script persistence and propagation in a core-team meeting and concluded we want them removed.
in any case i don't see a reason to keep script persistence if we trimmed the propagation part.

also, we'll need to remove the excessive command flags soon, but let's do it after #9656 is merged.

i added these to a todo section in the top comment.

@zhugezy i remind you about MASTER and SLAVE consistency with EVALSHA replication and possibly other tests that make no sense now.
on one side, as long as they pass, maybe they still have value and increase coverage, but on the other hand some may not make any sense.
specifically for the one mentioned above, i'd like to see it gone since it keeps hanging on freebsd CI.

@redis/core-team please approve (see top comment for a list of what's included and excluded)

Full CI: https://github.com/redis/redis/actions/runs/1603831303