kvrocks icon indicating copy to clipboard operation
kvrocks copied to clipboard

[CI][Flacky] TestList/BLMPOP_test_blocked_served_bothKey_FIFO_countOver_LEFT failed

Open mapleFU opened this issue 1 year ago • 2 comments

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!

mapleFU avatar Oct 22 '24 06:10 mapleFU

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 avatar Oct 31 '24 09:10 PokIsemaine

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.

PokIsemaine avatar Oct 31 '24 10:10 PokIsemaine