-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Add Support to 'SCRIPT KILL' Command #2158
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
Add Support to 'SCRIPT KILL' Command #2158
Conversation
/// <param name="flags">The command flags to use.</param> | ||
/// <returns>Simple string reply.</returns> | ||
/// <remarks><seealso href="https://redis.io/commands/script-kill/"/></remarks> | ||
void ScriptKill(CommandFlags flags = CommandFlags.None); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, I would expect a string response based on the docs - is that not the case here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I deleted the line from the documentation, I looked at the similar command: 'ScriptFlush' and there it was done this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shacharPash Sorry should have been clearer: I meant in the Redis server documentation, it returns a string so we should return it here :)
I have two concerns over SCRIPT KILL
1. That is won't actually work in our scenario: new connections most likely
won't be able to connect in the face of a blocking script, due to the
handshake operations we have - and existing connections are likely to be
stuck behind other operations, maybe just heartbeats
2. That it is a command that should almost never be used
Is this a "we have a problem where we need this" thing, or is it a "here's
a mm issuing command" thing? If the latter, I wonder whether on this
particular occasion: it might be better to stay missing
…On Fri, 17 Jun 2022, 00:00 Nick Craver, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/StackExchange.Redis/Interfaces/IServer.cs
<#2158 (comment)>
:
> @@ -448,6 +448,17 @@ public partial interface IServer : IRedis
/// <inheritdoc cref="ScriptLoad(LuaScript, CommandFlags)"/>
Task<LoadedLuaScript> ScriptLoadAsync(LuaScript script, CommandFlags flags = CommandFlags.None);
+ /// <summary>
+ /// Kills the currently executing EVAL script.
+ /// </summary>
+ /// <param name="flags">The command flags to use.</param>
+ /// <returns>Simple string reply.</returns>
+ /// <remarks><seealso href="https://redis.io/commands/script-kill/"/></remarks>
+ void ScriptKill(CommandFlags flags = CommandFlags.None);
@shacharPash <https://github.com/shacharPash> Sorry should have been
clearer: I meant in the Redis server documentation, it returns a string so
we should return it here :)
—
Reply to this email directly, view it on GitHub
<#2158 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAEHMAXXIM5DP6MN7CT4LTVPOWY5ANCNFSM5Y3U32PA>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***
com>
|
Since this doesn't make sense on a multiplexed connection, we're going to close this on out - it'd just almost never do what users expect. We can revisit this with connection pooling but it doesn't make sense the way connections work today. |
link to the command: https://redis.io/commands/script-kill/
part of #2055