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 for TYPE command for nonexistent objects #150

Merged
merged 6 commits into from
Apr 1, 2024

Conversation

cduluCNB
Copy link
Contributor

@cduluCNB cduluCNB commented Mar 26, 2024

This is a fix for a bug when running the TYPE command on a nonexistent, or recently expired object.

I was having an issue running RedisInsight and Another Redis Desktop Manager when I would add an object, and then it expired, both apps would hang and stop responding.

Using memurai-cli, for both Redis and Garnet, Redis normally returns "none" if it cannot find the object. Tested on Garnet v1.0.1, built in VS2022 in Release/Any CPU mode and using the .NET 8.0 GarnetServer, the session would just hang and I would have to Ctrl-C out of the session. According to the Generic API docs, it should also return "none".

When loading keys, RedisInsight and ARDM would run a SCAN command, then a TYPE command on each of the objects it would find. This is where Garnet would stop responding, causing the apps to also stop responding.

My proposed fix is to add a "none" status returned, and make sure the response string is generated even if the object is not found, so the Send() methods get called.

…r a response when TYPE is used on a non-existent object
@cduluCNB
Copy link
Contributor Author

@microsoft-github-policy-service agree

@cduluCNB
Copy link
Contributor Author

Also I wonder if this is a general bug - if dcurr is not modified, e.g. no simple string is appended to dcurr, then no response gets sent. Refer to RespServerSession.cs line 292.

RespServerSession Screenshot

@cduluCNB
Copy link
Contributor Author

My original tests were on Windows 11, using .NET 8.0.3 and VS 2022 Preview 17.10 Preview 1. I'm also able to replicate this using redis-cli with Garnet running on Ubuntu 22.04.3 LTS with .NET 8.0.3 in Release mode.

@vazois vazois added the API label Mar 27, 2024
@vazois
Copy link
Contributor

vazois commented Mar 27, 2024

Also I wonder if this is a general bug - if dcurr is not modified, e.g. no simple string is appended to dcurr, then no response gets sent. Refer to RespServerSession.cs line 292.

RespServerSession Screenshot

I don't think this is a real issue since we should be generating a response for every request. More like a safe guard right now to make sure that this is happening else client will hang.
We are working on revising the parser so this may or may not go away.

@TalZaccai TalZaccai requested a review from badrishc March 29, 2024 18:18
@badrishc badrishc merged commit b33189f into microsoft:main Apr 1, 2024
20 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jun 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants