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

fix: ignore LOADING error to prevent out of sync order message #719

Merged
merged 1 commit into from
Jan 10, 2025

Conversation

dntam00
Copy link
Contributor

@dntam00 dntam00 commented Jan 10, 2025

Fix #718
Treat LOADING server is loading the dataset in memory error from redis server as a PONG reply.

@rueian
Copy link
Collaborator

rueian commented Jan 10, 2025

Hi @dntam00,

Could you also duplicate the current test for NOPERM and change it to test the LOADING error:

rueidis/pipe_test.go

Lines 3243 to 3328 in 8897ec3

t.Run("PubSub unsubReply failed", func(t *testing.T) {
ctx := context.Background()
p, mock, cancel, _ := setup(t, ClientOption{})
commands := []Completed{
builder.Sunsubscribe().Channel("1").Build(),
builder.Sunsubscribe().Channel("2").Build(),
builder.Get().Key("mk").Build(),
}
replies := [][]RedisMessage{
{
{ // proactive unsubscribe before user unsubscribe
typ: '>',
values: []RedisMessage{
{typ: '+', string: "sunsubscribe"},
{typ: '+', string: "a"},
{typ: ':', integer: 0},
},
},
{ // proactive unsubscribe before user unsubscribe
typ: '>',
values: []RedisMessage{
{typ: '+', string: "sunsubscribe"},
{typ: '+', string: "b"},
{typ: ':', integer: 0},
},
},
}, {
// empty
}, {
{ // proactive unsubscribe after user unsubscribe
typ: '>',
values: []RedisMessage{
{typ: '+', string: "sunsubscribe"},
{typ: '_'},
{typ: ':', integer: 0},
},
},
{typ: '+', string: "mk"},
},
}
p.background()
// proactive unsubscribe before other commands
mock.Expect().Reply(RedisMessage{ // proactive unsubscribe before user unsubscribe
typ: '>',
values: []RedisMessage{
{typ: '+', string: "sunsubscribe"},
{typ: '+', string: "0"},
{typ: ':', integer: 0},
},
})
time.Sleep(time.Millisecond * 100)
for i, cmd1 := range commands {
cmd2 := builder.Get().Key(strconv.Itoa(i)).Build()
go func() {
if cmd1.IsUnsub() {
mock.Expect(cmd1.Commands()...).Expect(cmds.PingCmd.Commands()...).
Reply(replies[i]...).
Reply(RedisMessage{ // failed unsubReply
typ: '-',
string: "NOPERM User u has no permissions to run the 'ping' command",
}).Expect(cmd2.Commands()...).ReplyString(strconv.Itoa(i))
} else {
mock.Expect(cmd1.Commands()...).Reply(replies[i]...).Expect(cmd2.Commands()...).ReplyString(strconv.Itoa(i))
}
}()
if i == 2 {
if v, err := p.Do(ctx, cmd1).ToString(); err != nil || v != "mk" {
t.Fatalf("unexpected err %v", err)
}
} else {
if err := p.Do(ctx, cmd1).Error(); err != nil {
t.Fatalf("unexpected err %v", err)
}
}
if v, err := p.Do(ctx, cmd2).ToString(); err != nil || v != strconv.Itoa(i) {
t.Fatalf("unexpected val %v %v", v, err)
}
}
cancel()
})

@dntam00
Copy link
Contributor Author

dntam00 commented Jan 10, 2025

Hi @rueian,
I've updated the message of comment and a unit test.

@rueian rueian merged commit 5784e2f into redis:main Jan 10, 2025
2 checks passed
@rueian
Copy link
Collaborator

rueian commented Jan 10, 2025

Thank you @dntam00!

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

Successfully merging this pull request may close these issues.

CLUSTERDOWN response from redis cluster makes libary panic: protocol bug, message handled out of order
2 participants