[CI][Flacky] TestList/BLMPOP_test_blocked_served_bothKey_FIFO_countOver_LEFT failed

mapleFU commented 1 month ago

Search before asking

[X] I had searched in the issues and found no similar issues.

Version

--- FAIL: TestList (90.51s)
    --- FAIL: TestList/BLMPOP_test_blocked_served_bothKey_FIFO_countOver_LEFT#03 (0.20s)
        tcp_client.go:73: 
                Error Trace:    /home/runner/work/kvrocks/kvrocks/tests/gocase/util/tcp_client.go:73
                                            /home/runner/work/kvrocks/kvrocks/tests/gocase/util/tcp_client.go:100
                                            /home/runner/work/kvrocks/kvrocks/tests/gocase/unit/type/list/list_test.go:1525
                Error:          Not equal: 
                                expected: "blmpop-list2"
                                actual  : "blmpop-list1"

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1 +1 @@
                                -blmpop-list2
                                +blmpop-list1
                Test:           TestList/BLMPOP_test_blocked_served_bothKey_FIFO_countOver_LEFT#03
FAIL

See: https://github.com/apache/kvrocks/actions/runs/11444706001/job/31841260069?pr=2615

Minimal reproduce step

https://github.com/apache/kvrocks/actions/runs/11444706001/job/31841260069?pr=2615

What did you expect to see?

Pass

What did you see instead?

See above

Anything Else?

no

Are you willing to submit a PR?

[ ] I'm willing to submit a PR!

PokIsemaine commented 3 weeks ago

I'm guessing that's because the WriteArgs command was actually accepted by the server later than the next two RPUSHs

require.NoError(t, rdb.RPush(ctx, key2, "one", "two").Err()) // 1
require.NoError(t, rdb.RPush(ctx, key1, "ONE", "TWO").Err()) // 2
require.NoError(t, rd.WriteArgs("blmpop", "1", "2", key1, key2, direction, "count", "2")) // 3

The most simple solution is to add a sleep, but this can be confusing in testing, because some WriteArgs don't require the code to be executed exactly in the order of the code, as long as it has been executed, and sleep is not required.

We may need a clearer way to articulate our testing expectations, what do you think?

PokIsemaine commented 3 weeks ago

I'd like to eliminate sleep from the test as much as possible, or add comments to sleep.

Keep and add comments

Temporarily patch for known issues, e.g.: https://github.com/apache/kvrocks/issues/2473
You really need to use sleep to ensure the order of command execution

Identify and eliminate: Unnecessary sleep

Identify and eliminate/ add comments: Unclear why sleep is needed. They might be hiding some issues. We might need to clear them all first and then determine whether we need to sleep and why we need to do so.

apache / kvrocks