Skip to content
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

[CI][Flacky] TestList/BLMPOP_test_blocked_served_bothKey_FIFO_countOver_LEFT failed #2617

Open
1 of 2 tasks
mapleFU opened this issue Oct 22, 2024 · 2 comments
Open
1 of 2 tasks
Labels
bug type bug

Comments

@mapleFU
Copy link
Member

mapleFU commented Oct 22, 2024

Search before asking

  • 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 mapleFU added the bug type bug label Oct 22, 2024
@PokIsemaine
Copy link
Contributor

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
Copy link
Contributor

PokIsemaine commented Oct 31, 2024

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

Keep and add comments

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug type bug
Projects
None yet
Development

No branches or pull requests

2 participants