garnet icon indicating copy to clipboard operation
garnet copied to clipboard

API Coverage - Implement SINTER and SINTERSTORE

Open TalZaccai opened this issue 1 year ago • 13 comments
trafficstars

Here's your opportunity to implement a couple of RESP commands in Garnet!

SINTER

Syntax: SINTER key [key ...]

Command description: Returns the members of the set resulting from the intersection of all the given sets.

For example:

key1 = {a,b,c,d}
key2 = {c}
key3 = {a,c,e}
SINTER key1 key2 key3 = {c}

Keys that do not exist are considered to be empty sets. With one of the keys being an empty set, the resulting set is also empty (since set intersection with an empty set always results in an empty set).

Response:

Array reply: a list with members of the resulting set.

SINTERSTORE

Syntax: SINTERSTORE destination key [key ...]

Command description: This command is equal to SINTER, but instead of returning the resulting set, it is stored in destination.

If destination already exists, it is overwritten.

Response:

Integer reply: the number of elements in the resulting set.


How to approach this task:

  1. Add the new command info to the setCommandsInfoMap 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/SetOps.cs)
  5. Define a new method in IGarnetAPI 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/SetCommands.cs)
  7. Add a new method to SetObjImpl which will perform the required operation on the hash object (Garnet.server/Objects/Set/SetObjectImpl.cs)
  8. Add a new enum value to SetOperation and add the appropriate case to the switch in SetObject.Operate, in which you will call the method defined in step #8 (Garnet.server/Objects/Set/SetObject.cs)
  9. Add an appropriate case to the switch in TransactionManager.SetObjectKeys (Garnet.server/Transaction/TxnKeyManager.cs)
  10. Add tests for the new command in the RespSetTests class (Garnet.test/RespSetTests.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!

TalZaccai avatar Mar 28 '24 20:03 TalZaccai

I'm interested in taking a stab at this!

jlao avatar Mar 30 '24 02:03 jlao

I'm interested in taking a stab at this!

Go ahead! LMK if you have any questions!

TalZaccai avatar Mar 30 '24 02:03 TalZaccai

@TalZaccai , can you please update the assignees? This will help others easily find the unpicked items. If this one has not been picked up yet, I'm interested.

MojtabaTajik avatar Apr 08 '24 05:04 MojtabaTajik

If more than one person wants to attempt an open work item, please go ahead. If a PR comes in quicker and meets the requirements, then we would be happy to merge it.

badrishc avatar Apr 10 '24 19:04 badrishc

Someone else can take this. I don't think I have time to tackle this in a timely manner right now.

jlao avatar Apr 11 '24 20:04 jlao

@MojtabaTajik Let me know if you'd like to try and tackle this one!

TalZaccai avatar Apr 12 '24 02:04 TalZaccai

@TalZaccai yes, pls assign it to me.

MojtabaTajik avatar Apr 12 '24 05:04 MojtabaTajik

@TalZaccai yes, pls assign it to me.

Done! :)

TalZaccai avatar Apr 12 '24 05:04 TalZaccai

@MojtabaTajik - any update on this?

badrishc avatar Apr 22 '24 22:04 badrishc

@TalZaccai @badrishc

I just started working on it this week. Since it's my first contribution to Garnet, it's taking me more time to understand everything.

I've completed all the steps, but in step 7, there's a partial class called "SetObject" which contains a HashSet<byte[]> field named "set". I've written the "SetIntersect" method there, allowing me to retrieve set names requested by the user (setsToIntersect variable). I also have the current set in the "set" field. However, I'm not sure how to retrieve other set members here and perform the intersection operation.

Am I proceeding correctly here?

        private void SetIntersect(byte* input, int length, ref SpanByteAndMemory output)
        {
            var _input = (ObjectInputHeader*)input;
            int numSets = _input->count;

            bool isMemory = false;
            MemoryHandle ptrHandle = default;
            
            // Parse the input to get the sets you want to intersect
            List<HashSet<byte[]>> setsToIntersect = new List<HashSet<byte[]>>();
            byte* inputCurrPtr = input + sizeof(ObjectInputHeader);
            for (int i = 0; i < numSets; i++)
            {
                if (!RespReadUtils.ReadByteArrayWithLengthHeader(out var setBytes, ref inputCurrPtr, input + length))
                    return; // Handle parsing error

                var tempSect = new HashSet<byte[]>() { setBytes };
                setsToIntersect.Add(tempSect);
            }

            // Perform the intersection
            var currentSet = set;
            
        }

MojtabaTajik avatar Apr 23 '24 04:04 MojtabaTajik

@TalZaccai @badrishc

I just started working on it this week. Since it's my first contribution to Garnet, it's taking me more time to understand everything.

I've completed all the steps, but in step 7, there's a partial class called "SetObject" which contains a HashSet<byte[]> field named "set". I've written the "SetIntersect" method there, allowing me to retrieve set names requested by the user (setsToIntersect variable). I also have the current set in the "set" field. However, I'm not sure how to retrieve other set members here and perform the intersection operation.

Am I proceeding correctly here?

        private void SetIntersect(byte* input, int length, ref SpanByteAndMemory output)
        {
            var _input = (ObjectInputHeader*)input;
            int numSets = _input->count;

            bool isMemory = false;
            MemoryHandle ptrHandle = default;
            
            // Parse the input to get the sets you want to intersect
            List<HashSet<byte[]>> setsToIntersect = new List<HashSet<byte[]>>();
            byte* inputCurrPtr = input + sizeof(ObjectInputHeader);
            for (int i = 0; i < numSets; i++)
            {
                if (!RespReadUtils.ReadByteArrayWithLengthHeader(out var setBytes, ref inputCurrPtr, input + length))
                    return; // Handle parsing error

                var tempSect = new HashSet<byte[]>() { setBytes };
                setsToIntersect.Add(tempSect);
            }

            // Perform the intersection
            var currentSet = set;
            
        }

Hey @MojtabaTajik!

I understand your confusion, I think originally when I wrote these instructions they were meant for adding a single-object operation, so given your input I might update those for future work items :)

To your question, your parsing logic should go into SetCommands.cs and the main command logic would go into SetOps.cs, where you can obtain the appropriate locks on the relevant keys and you could call the StorageSession's GET and SET operations to read & write to & from the object store.

I would recommend that you review some similar commands that were implemented recently, such as LMOVE, SDIFF, SDIFFSTORE & SUNION for reference.

Thanks again for your question, Don't hesitate to reach out again.

Tal.

TalZaccai avatar Apr 23 '24 05:04 TalZaccai

@TalZaccai I was working on this on my weekends. I just saw a PR from Lapkarol that will implement them so you can use them.

MojtabaTajik avatar Apr 28 '24 06:04 MojtabaTajik

@TalZaccai I was working on this on my weekends. I just saw a PR from Lapkarol that will implement them so you can use them.

Sounds good, I'll have a look at that new PR. Didn't know that someone else was working on it...

TalZaccai avatar Apr 30 '24 18:04 TalZaccai