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

get vs gets #130

Open
brayniac opened this issue Apr 1, 2022 · 2 comments
Open

get vs gets #130

brayniac opened this issue Apr 1, 2022 · 2 comments

Comments

@brayniac
Copy link

brayniac commented Apr 1, 2022

It appears that the get and gets commands implemented in this crate differ slightly from the protocol

The get method should take one or more keys. The gets method is about returning the current CAS value as part of the retrieval command and also operates on one or more keys.

Please see the memcache protocol spec for the definition of these commands.

If I'm understanding the implementation correctly, the gets method is correct and should also retrieve the CAS values, but the get method is not fully implemented as it should support requesting more than one key in a single call. I believe the current docs mistakenly direct users towards the gets function for multi-key retrieval even though that is not the difference between get and gets.

@aisk
Copy link
Owner

aisk commented May 16, 2023

Yes, we are using a wrong name for multiple gets.

But I think the memcache protocol command name may not be the same as the client library function name. For example, implement a get function which can called with a list of keys in a static typed language, especially a one without function overload is hard and not best practice.

So we should have three functions:

  • Get a single key;
  • Get multiple keys;
  • Get a single key with CAS token.

But we wrongly used the gets name for "get multiple keys", for compatibility issue, maybe we should keep it and use a different name like get_with_cas for "get a single key with CAS token".

@brayniac
Copy link
Author

It's been a while since I looked at this (over a year) so might get some details wrong by just lightly refreshing my memory.

The documentation for cas(...) directs people to use gets(...) to retrieve the cas value. Which is correct in the protocol sense as this client library command executes a GETS .... But it's also the only way to do a multi-key get currently.

If it's intended to only be a multi-key get (eg: mget for Redis) then it should not retrieve the cas value. On some server implementations there may be additional costs to retrieve the cas value, and it's more bytes on the wire for the response (and the cost to serialize as a text representation and parse it again).

The only option that maintains compatibility would be leaving gets(...) as-is and creating new mget(...) using GET, get_with_cas(...), and mget_with_cas(...) (both using GETS). Unfortunately this allows people to continue to use an unnecessarily expensive implementation for multi-key get and single key get with cas (as the current gets(...) may be used for either case and is not ideal for either single key GETS or multi-key GET). For performance reasons, it seems that current users should eventually make a change in their code to avoid the current gets(..) command.

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

No branches or pull requests

2 participants