search

valkey-io / valkey-glide

An open source Valkey client library that supports Valkey and Redis open source 6.2, 7.0 and 7.2. Valkey GLIDE is designed for reliability, optimized performance, and high-availability, for Valkey and Redis OSS based applications. GLIDE is a multi language client library, written in Rust with programming language bindings, such as Java and Python
Apache License 2.0
281 stars 58 forks source link

Python: FAILED test_async_client.py::TestScripts::test_script_kill_route #2617

Open ikolomi opened 3 weeks ago

ikolomi commented 3 weeks ago

Describe the bug

CI links: https://github.com/valkey-io/valkey-glide/actions/runs/11698386348/job/32578606911#step:6:57 https://github.com/valkey-io/valkey-glide/actions/runs/11698386348/job/32578623343#step:6:57 https://github.com/valkey-io/valkey-glide/actions/runs/11698386348/job/32578624371#step:6:56 https://github.com/valkey-io/valkey-glide/actions/runs/11698386348/job/32578624993#step:6:57 https://github.com/valkey-io/valkey-glide/actions/runs/11698386348/job/32578628878#step:6:57 https://github.com/valkey-io/valkey-glide/actions/runs/11698386348/job/32578634733#step:6:56 https://github.com/valkey-io/valkey-glide/actions/runs/11698386348/job/32578637153#step:6:57 https://github.com/valkey-io/valkey-glide/actions/runs/11698386348/job/32578638394#step:6:56 https://github.com/valkey-io/valkey-glide/actions/runs/11698386348/job/32578641767#step:6:58 https://github.com/valkey-io/valkey-glide/actions/runs/11698386348/job/32578651731#step:6:56

Expected Behavior

test pass

Current Behavior

test fail

Reproduction Steps

manual run or CI

Possible Solution

No response

Additional Information/Context

No response

Client version used

release-1.2

Engine type and version

.

OS

.

Language

Python

Language Version

.

Cluster information

No response

Logs

No response

Other information

No response

shohamazon commented 3 weeks ago

Script kill command info:

2)  1) "script|kill"
           2) (integer) 2
           3) 1) noscript
              2) allow_busy
           4) (integer) 0
           5) (integer) 0
           6) (integer) 0
           7) 1) @slow
              2) @scripting
           8) 1) "request_policy:all_shards"
              2) "response_policy:one_succeeded"

since the response policy is one_succeeded, we might not kill the script from all running nodes (when we run with all nodes route) but we do kill from some. So for example, we have 3 shards, 2 returned an OK response, one didnt, the whole command returned OK but we still have a running script in progress.

acarbonetto commented 3 weeks ago

@shohamazon would it work to direct all work to one specific shard?

shohamazon commented 3 weeks ago

@shohamazon would it work to direct all work to one specific shard?

You mean having a single route? Because we already have a test with a single route and looks like it works fine

shohamazon commented 3 weeks ago

@note : disabled for now for green CI, when fixed, need to be added back #2631

acarbonetto commented 3 weeks ago

@shohamazon we should add a TODO to fix this later. We can do the same disabling for Node and Java.

shohamazon commented 3 weeks ago

@shohamazon we should add a TODO to fix this later.

We can do the same disabling for Node and Java.

@acarbonetto I believe this whole issue is a TODO