search

microsoft / garnet

Garnet is a remote cache-store from Microsoft Research that offers strong performance (throughput and latency), scalability, storage, recovery, cluster sharding, key migration, and replication features. Garnet can work with existing Redis clients.
https://microsoft.github.io/garnet/
MIT License
9.99k stars 484 forks source link

API Coverage - Implement LMPOP #273

Closed TalZaccai closed 3 weeks ago

TalZaccai commented 4 months ago

Here's your opportunity to implement a RESP command in Garnet!

LMPOP

Syntax: LMPOP numkeys key [key ...] <LEFT | RIGHT> [COUNT count]

Command description:

Pops one or more elements from the first non-empty list key from the list of provided key names.

Elements are popped from either the left or right of the first non-empty list based on the passed argument. The number of returned elements is limited to the lower between the non-empty list's length, and the count argument (which defaults to 1).

Response:

One of the following:

How to approach this task:

  1. Add the new command info to the listCommandsInfoMap in the RespCommandsInfo class (Garnet.server/Resp/RespCommandsInfo.cs)
  2. Add the new command string to the returned HashSet in the RespInfo.GetCommands method (Garnet.server/Resp/RespInfo.cs)
  3. Add the appropriate fast parsing logic for the new command in the RespCommand.FastParseArrayCommand method (use other commands as reference) (Garnet.server/Resp/RespCommand.cs)
  4. Implement the wrapper method to the storage layer in StorageSession (Garnet.server/Storage/Session/ObjectStore/ListOps.cs)
  5. Define a new method in IGarnetAPI or IGarnetReadAPI and implement it in GarnetApiObjectCommands, calling the method you have implemented in step #4 (Garnet.server/API/IGarnetAPI.cs, Garnet.server/API/GarnetApiObjectCommands.cs)
  6. Add a new method to the RespServerSession class which will call the storage API and return the appropriate response (Garnet.server/Resp/Objects/ListCommands.cs)
  7. Add a new method to ListObjImpl which will perform the required operation on the list object (Garnet.server/Objects/List/ListObjectImpl.cs)
  8. Add a new enum value to ListOperation and add the appropriate case to the switch in ListObject.Operate, in which you will call the method defined in step #8 (Garnet.server/Objects/List/ListObject.cs)
  9. Add an appropriate case to the switch in TransactionManager.ListObjectKeys (Garnet.server/Transaction/TxnKeyManager.cs)
  10. Add tests for the new command in the RespListTests class (Garnet.test/RespListTests.cs)

Tip: First add a simple test that calls the new command and use it to debug as you develop, it will make your life much easier!

If you have any questions, the Garnet team is here to help!

Zombach commented 4 months ago

@TalZaccai I'm interested in taking a stab at this!

TalZaccai commented 4 months ago

@TalZaccai I'm interested in taking a stab at this!

Go for it! LMK if you have any questions!

TalZaccai commented 2 months ago

@Zombach any progress here? :)

Zombach commented 2 months ago

@Zombach any progress here? :) Hi I've been busy with this Add Garnet in Aspire

I'll do it this weekend. On Monday I’ll post what came of it. Or it didn't work out.

Zombach commented 2 months ago

Unfortunately, I don't have time for this task. Hope someone else implements this.

funcmike commented 1 month ago

@TalZaccai I can do this. I'll write in a few days how I'm doing.

TalZaccai commented 1 month ago

@TalZaccai I can do this. I'll write in a few days how I'm doing.

Great! Looking forward to hearing how you faired! :)

funcmike commented 1 month ago

@TalZaccai Ok so basic support is working. Can you please review my code? I also have questions about:

  1. Add a new method to ListObjImpl which will perform the required operation on the list object (Garnet.server/Objects/List/ListObjectImpl.cs)
  2. Add a new enum value to ListOperation and add the appropriate case to the switch in ListObject.Operate, in which you will call the method defined in step #8 (Garnet.server/Objects/List/ListObject.cs)
  3. Add an appropriate case to the switch in TransactionManager.ListObjectKeys (Garnet.server/Transaction/TxnKeyManager.cs)

LMPOP is a operation on many lists when it's just on one list it's the same as LPOP/RPOP (which are more straightforward imho). For now I skipped it because I don't get it what's the point of implementing it in these files - maybe I'm miss understand something?

TalZaccai commented 1 month ago

@TalZaccai Ok so basic support is working. Can you please review my code? I also have questions about:

  1. Add a new method to ListObjImpl which will perform the required operation on the list object (Garnet.server/Objects/List/ListObjectImpl.cs)
  2. Add a new enum value to ListOperation and add the appropriate case to the switch in ListObject.Operate, in which you will call the method defined in step #8 (Garnet.server/Objects/List/ListObject.cs)
  3. Add an appropriate case to the switch in TransactionManager.ListObjectKeys (Garnet.server/Transaction/TxnKeyManager.cs)

LMPOP is a operation on many lists when it's just on one list it's the same as LPOP/RPOP (which are more straightforward imho). For now I skipped it because I don't get it what's the point of implementing it in these files - maybe I'm miss understand something?

@funcmike You are correct, those are only relevant for single-key operations (my apologies, I copy & pasted this explanation from a former task)

I'll review your code today :) Thanks for your contribution!

edit: You do need to add it to the TxnKeyManager though, since it can be called within the context of a transaction