Hello Hitesh,

The following is my feedback.

*1. Redis Type String*
  I like the idea of creating a region upfront. If we are still using the
convention that internal region names start with "__" , then I would
suggest something like a region named "__RedisString region"

*2. List Type*

I propose using a single partition region (ex: "__RedisList") for the List
commands.

Region<byteArrayWrapper,ArrayList<byteArrayWrapper>> region;

//Note: ByteArrayWrapper is what the current RedisAdapter uses as its data
type. It converts strings to bytes using UTF8 encoding

Example Redis commands

RPUSH mylist A =>

 Region<ByteArrayWrapper,List<ByteArrayWrapper>> region =
getRegion("__RedisList")
 List list = getOrCreateList(mylist);
 list.add(A)
 region.put(mylist,list)

*3. Hashes*

Based on my Spring Data Redis testing for Hash/object support.

HMSET and similar Hash commands are submitted in the following format:
HMSET region:key [field value]+ I proposed creating regions with the
following format:

Region<ByteArrayWrapper,Map<ByteArrayWrapper,ByteArrayWrapper>> region;

Also see Hashes section at the following URL https://redis.io/topics/
data-types

Example Redis command:

HMSET companies:100 _class io.pivotal.redis.gemfire.example.repository.Company
id 100 name nylaInc email i...@nylainc.io website nylaInc.io taxID id:1
address.address1 address1 address.address2 address2 address.cityTown
cityTown address.stateProvince stateProvince address.zip zip
address.country country

=>

//Pseudo Access code
Region<ByteArrayWrapper,Map<ByteArrayWrapper,ByteArrayWrapper>>
companiesRegion = getRegion("companies")
companiesRegion.put(100, toMap(fieldValues))

//------

// HGETALL region:key

HGETALL companies:100 =>

Region<key,Map<field,value>> companiesRegion = getRegion("companies")
return companiesRegion.get(100)

//HSET region:key field value

HSET companies:100 email upda...@pivotal.io =>

Region<key,Map<field,value>> companiesRegion = getRegion("companies");
Map map = companiesRegion.get(100)
map.set(email,upda...@pivotal.io)
companiesRegion.put(100,map);

FYI - I started to implement this and hope to submit a pull request soon
related to GEODE-2469.


*4. Set*

I propose using a single partition region (ex: __RedisSET) for the SET
commands.

Region<byteArrayWrapper,HashSet<byteArrayWrapper>> region;

Example Redis commands

SADD myset "Hello" =>

Region<ByteArrayWrapper,Set<ByteArrayWrapper>> region = getRegion("__RedisSET");
Set set = region(myset)
boolean bool = set.add(Hello)
if(bool){
  region.put(myset,set)
}
return bool;

SMEMBERS myset "Hello" =>

    Region<ByteArrayWrapper,Set<ByteArrayWrapper>> region =
getRegion("_RedisSET");
        Set set = region(myset)
        return set.contains(Hello)s

FYI - I started to implement this and hope to submit a pull request soon
related to GEODE-2469.


*5. SortedSets *

I propose using a single partition region for the SET commands.

Region<byteArrayWrapper,TreeSet<byteArrayWrapper>> region;

6. Default config for geode-region (vote)

I think the default setting should be partitioned with persistence and no
redundant copies.

7. It seems; redis knows type(list, hashes, string ,set ..) of each key...

I suggested most operations can assume all keys are strings in UTF8 byte
encoding, not sure if there are any mathematical number based Redis
commands that need numbers.

*8. Transactions:*

+1 I agree to not support transactions

*9. Redis COMMAND* (https://redis.io/commands/comman
<https://redis.io/commands/command>

+1 for implementing the "COMMAND"


---------- Forwarded message ----------
From: Hitesh Khamesra <hitesh...@yahoo.com.invalid>
Date: Tue, Feb 14, 2017 at 5:36 PM
Subject: GeodeRedisAdapter improvments/feedback
To: Geode <dev@geode.apache.org>, "u...@geode.apache.org" <
u...@geode.apache.org>


Current GeodeRedisAdapter implementation is based on
https://cwiki.apache.org/confluence/display/GEODE/Geode+Redi
s+Adapter+Proposal.
We are looking for some feedback on Redis commands and their mapping to
geode region.

1. Redis Type String
  a. Usage Set k1 v1
  b. Current implementation creates "STRING_REGION" geode-partition-region
upfront
  c. This k1/v1 are geode-region key/value
  d. Any feedback?

2. List Type
  a. usage "rpush mylist A"
  b. Current implementation maps each list to geode-partition-region(i.e.
mylist is geode-partition-region); with the ability to get item from
head/tail
  c. Feedback/vote
      -- List type operation at region-entry level;
      -- region-key = "mylist"
      -- region-value = Arraylist (will support all redis list ops)
  d. Feedback/vote: both behavior is desirable


3. Hashes
  a. this represents field-value or java bean object
  b. usage "hmset user1000 username antirez birthyear 1977 verified 1"
  c. Current implementation maps each hashes to geode-partition-region(i.e.
user1000 is geode-partition-region)
  d. Feedback/vote
    -- Should we map hashes to region-entry
    -- region-key = user1000
    -- region-value = map
    -- This will provide java bean sort to behaviour with 10s of field-value
    -- Personally I would prefer this..
  e. Feedback/vote: both behaviour is desirable

4. Sets
  a. This represents unique keys in set
  b. usage "sadd myset 1 2 3"
  c. Current implementation maps each sadd to geode-partition-region(i.e.
myset is geode-partition-region)
  d. Feedback/vote
    -- Should we map set to region-entry
    -- region-key = myset
    -- region-value = Hashset
  e. Feedback/vote: both behaviour is desirable

5. SortedSets
  a. This represents unique keys in set with score (usecase Query top-10)
  b. usage "zadd hackers 1940 "Alan Kay""
  c. Current implementation maps each zadd to geode-partition-region(i.e.
hackers is geode-partition-region)
  d. Feedback/vote
    -- Should we map set to region-entry
    -- region-key = hackers
    -- region-value = Sorted Hashset
  e. Feedback/vote: both behaviour is desirable

6. HyperLogLogs
  a. A HyperLogLog is a probabilistic data structure used in order to count
unique things (technically this is referred to estimating the cardinality
of a set).
  b. usage "pfadd hll a b c d"
  c. Current implementation creates "HLL_REGION" geode-partition-region
upfront
  d. hll becomes region-key and value is HLL object
  e. any feedback?

7. Default config for geode-region (vote)
   a. partition region
   b. 1 redundant copy
   c. Persistence
   d. Eviction
   e. Expiration
   f. ?

8. It seems; redis knows type(list, hashes, string ,set ..) of each key.
Thus for each operation we need to make sure type of key. In current
implementation we have different region for each redis type. Thus we have
another region(metaTypeRegion) which keeps type for each key. This makes
any operation in geode slow as it needs to verify that type. For instance,
creating new key need to make sure its already there or not. Whether we
should allow type change or not.
  a. Feedback/vote
     -- type change of key
     -- Can we allow two key with same name but two differnt type (as it
will endup in two different geode-region)
        String type "key1" in string region
        HLL type "key1" in HLL region
  b. any other feedback

9. Transactions:
  a. we will not support transaction in redisAdapter as geode transaction
are limited to single node.
  b. feedback?

10. Redis COMMAND (https://redis.io/commands/command)
  a. should we implement this "COMMAND" ?

11. Any other redis command we should consider?


Thanks.Hitesh




-- 
*Gregory Green* (Senior Data Engineer)
ggr...@pivotal.io

Reply via email to