Support GEO #59

Leviathan1995 · 2017-02-15T20:18:52Z

添加GEO支持，其中geohash算法使用redis中的src/geohash.h, src/geohash.c, src/geohash_helper.h, src/geohash_helper.c , 因为nemo中zset数据结构的score精度只有13位, 所以对redis中的geohash算法的精度step做了修改。

因为GEO直接依赖于zset数据结构，所以只在pika层进行了命令的添加, 未对nemo做任何修改。

目前添加了4条命令, GEOADD, GEOPOS, GEODIST, GEOHASH.

leviathan@ubuntu:~/Documents/redis/src$ ./redis-benchmark -p 9221 -t geoadd geopos geodist geohash -r 10000 -n 10000
====== geopos geodist geohash -r 10000 -n 10000 ======
  100000 requests completed in 3.66 seconds
  50 parallel clients
  3 bytes payload
  keep alive: 1

0.00% <= 1 milliseconds
90.47% <= 2 milliseconds
97.73% <= 3 milliseconds
98.74% <= 4 milliseconds
99.24% <= 5 milliseconds
99.67% <= 6 milliseconds
99.70% <= 7 milliseconds
99.77% <= 8 milliseconds
99.89% <= 9 milliseconds
99.90% <= 10 milliseconds
99.90% <= 11 milliseconds
99.95% <= 13 milliseconds
99.96% <= 14 milliseconds
99.97% <= 15 milliseconds
99.97% <= 16 milliseconds
99.97% <= 17 milliseconds
99.97% <= 18 milliseconds
99.98% <= 19 milliseconds
99.98% <= 20 milliseconds
99.98% <= 26 milliseconds
99.98% <= 27 milliseconds
99.99% <= 28 milliseconds
100.00% <= 30 milliseconds
100.00% <= 31 milliseconds
100.00% <= 32 milliseconds
100.00% <= 33 milliseconds
100.00% <= 34 milliseconds
100.00% <= 36 milliseconds
27322.40 requests per second

CatKang · 2017-02-16T06:48:45Z

include/pika_command.h 中的CmdFlags是记录了命令的各种属性，会和CmdFlagsMask做位运算。其中低2，3，4位是记录命令类型的，所以目前只能有八种。 kCmdFlagsGeo = 18, 这样定义会被解析为kCmdFlagsLocal | kCmdFlagsKv

可以多分配几位给命令类型，将后边的其他属性的位后移吧。

Leviathan1995 · 2017-02-16T09:40:53Z

@CatKang 我计划将第5位分配给命令类型, 目前其它属性有4个, 即存放在32,64,128,255

Leviathan1995 · 2017-02-16T18:54:36Z

添加命令GEORADIUS, GEORADIUSBYMEMBER.
简单的测试用例:

127.0.0.1:9221> GEOADD Sicily 13.361389 38.115556 "Palermo" 15.087269 37.502669 "Catania"
(integer) 2
127.0.0.1:9221> GEORADIUS Sicily 15 37 200 km WITHDIST WITHCOORD WITHHASH asc
1) 1) "Catania"
   2) "56.465174352955216"
   3) 1) "15.087661743164062"
      2) "37.502842051731029"
   4) "53092153485"
2) 1) "Palermo"
   2) "190.42765439947635"
   3) 1) "13.361434936523438"
      2) "38.115392905981523"
   4) "53086852359"
127.0.0.1:9221> GEORADIUS Sicily 15 37 200 km WITHDIST WITHCOORD WITHHASH count 1
1) 1) "Catania"
   2) "56.465174352955216"
   3) 1) "15.087661743164062"
      2) "37.502842051731029"
   4) "53092153485"
127.0.0.1:9221> GEOADD Sicily 13.583333 37.316667 "Agrigento"
(integer) 1
127.0.0.1:9221> GEORADIUSBYMEMBER Sicily Agrigento 100 km
1) "Agrigento"
2) "Palermo"
127.0.0.1:9221> GEORADIUS Sicily 15 37 10 km
(empty list or set)

CatKang · 2017-02-19T04:20:10Z

include/pika_command.h

+  kCmdFlagsMaskLocal            = 32,
+  kCmdFlagsMaskSuspend          = 64,
+  kCmdFlagsMaskPrior            = 128,
+  kCmdFlagsMaskAdminRequire     = 255


kCmdFlagsMaskAdminRequire 这个地方应该是256

@CatKang 已修改.

kCmdFlagsMaskAdminRequire 和 kCmdFlagsAdminRequire 都应该是256

kCmdFlagsMaskAdminRequire 是掩码，会跟Flag做位与，来获取第9位是不是为1

CatKang · 2017-02-19T04:21:52Z

src/pika_command.cc

+  CmdInfo* geoposptr = new CmdInfo(kCmdNameGeoPos, -3, kCmdFlagsRead | kCmdFlagsGeo);
+  cmd_infos.insert(std::pair<std::string, CmdInfo*>(kCmdNameGeoPos, geoposptr));
+  ////GeoDist
+  CmdInfo* geodistptr = new CmdInfo(kCmdNameGeoDist, -3, kCmdFlagsWrite | kCmdFlagsGeo);


geodist需要是kCmdFlagsWrite吗，write命令是改变库状态的命令，会通过binlog同步到所有的从。

@CatKang 已修改，GEODIST需要kCmdFlagsRead.

KernelMaker · 2017-02-19T04:35:29Z

@Leviathan1995 还需要测试下和redis GEO相同命令不同case下返回值是否一样，一定要和redis用法及返回结果兼容，或明确指出无法做到100%兼容的case，将来方便用户从redis GEO迁移到pika GEO之前进行评估

Leviathan1995 · 2017-02-19T15:04:00Z

@KernelMaker 我现在人工测试了一遍，与redis协议兼容，是否有更合理的测试方法？

CatKang · 2017-02-20T03:06:46Z

src/pika_geo.cc

+  	std::string str_bits = std::to_string(bits);
+  	double previous_score, score;
+  	slash::string2d(str_bits.data(), str_bits.size(), &score);
+  	s = db->ZScore(key_, iter->member, &previous_score);


这里是不是可以直接ZAdd，而不用先ZScore在ZAdd，ZAdd会判断是不是存在，最后一个出参ret可以拿到新增的个数。另外，最好能把格式对其整理下

CatKang · 2017-02-20T03:42:14Z

src/pika_command.cc

+  cmd_infos.insert(std::pair<std::string, CmdInfo*>(kCmdNameGeoRadius, georadiusptr));
+  ////GeoRadiusByMember
+  CmdInfo* georadiusbymemberptr = new CmdInfo(kCmdNameGeoRadiusByMember, -3, kCmdFlagsRead | kCmdFlagsGeo);
+  cmd_infos.insert(std::pair<std::string, CmdInfo*>(kCmdNameGeoRadiusByMember, georadiusbymemberptr));


这里CmdInfo代表应该有的参数个数，包括命令名在内，负数代表最少参数个数，会在Cmd DoIntial时检查

几个命令的这个值是不是有问题，比如geoadd 应该是-5，geoadd key longitude latitude member； geopos应该是-3; geodist -4; geohash -3; georadius -6; georadiusbymember -5

geoadd参数最少个数为-5
geopos参数最少个数为-2

127.0.0.1:6379> geoadd xj 1.0 2.0 kt (integer) 0 127.0.0.1:6379> geopos xj (empty list or set)

geodist参数最少个数为-4
geohash参数最少个数为-2

127.0.0.1:6379> geohash xj (empty list or set)

georadius参数最少个数为-6
georadiusbymember参数最少个数为-5

CatKang · 2017-02-20T05:17:29Z

src/pika_geo.cc

+  }
+  size_t argc = argv.size();
+  if (argc < 6) {
+  	res_.SetRes(CmdRes::kWrongNum, kCmdNameGeoRadius);


这步判断可以放到上面的CheckArg，将georeaius命令的CmdInfo第二个参数配成-6.

GeoRadiusByMemberCmd中也是一样

CatKang · 2017-02-20T05:27:21Z

include/pika_command.h

+  kCmdFlagsMaskLocal            = 32,
+  kCmdFlagsMaskSuspend          = 64,
+  kCmdFlagsMaskPrior            = 128,
+  kCmdFlagsMaskAdminRequire     = 255


kCmdFlagsMaskAdminRequire 和 kCmdFlagsAdminRequire 都应该是256

kCmdFlagsMaskAdminRequire 是掩码，会跟Flag做位与，来获取第9位是不是为1

CatKang · 2017-02-20T05:38:36Z

关于测试，redis/tests/unit下面有redis的接口测试，可以用这个跑下试试。可以的话麻烦给pika也添加一下吧.

pika/pikatests/TESTREAD 这个里面有pika的测试说明

可以先将redis的geo测试脚本添加到pika/pikatests/tests下面，然后按TESTREAD这个说明跑起来

CatKang · 2017-02-21T01:29:29Z

感谢贡献geo代码，有时间可以完善下测试脚本和wiki中pika 支持的redis接口及兼容情况中的内容

…ation#59)

add binlog when migrate

leviathan added 3 commits February 16, 2017 03:40

Support GEO

399a7cf

format code

caa8708

redis protocol compatible

408e9cd

Support GEORADIUS AND GEORADIUSBYMEMBER

c78502b

fix warn

46ce746

CatKang reviewed Feb 19, 2017

View reviewed changes

leviathan added 2 commits February 19, 2017 15:05

modify flags

eadd063

compatible with redis protocol

095579d

CatKang reviewed Feb 20, 2017

View reviewed changes

bugfix

1347b55

CatKang merged commit 6131657 into OpenAtomFoundation:master Feb 21, 2017

Leviathan1995 mentioned this pull request Feb 26, 2017

How about GEO support #13

Closed

AlexStocks pushed a commit to ipixiu/pika that referenced this pull request Dec 21, 2018

complete the common interface in Lists for type of key (OpenAtomFound…

206685e

…ation#59)

luky116 added a commit to luky116/pika that referenced this pull request Jun 16, 2023

feat: add binlog when migrate (OpenAtomFoundation#59)

0da527a

add binlog when migrate

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support GEO #59

Support GEO #59

Leviathan1995 commented Feb 15, 2017

CatKang commented Feb 16, 2017

Leviathan1995 commented Feb 16, 2017 •

edited

Loading

Leviathan1995 commented Feb 16, 2017 •

edited

Loading

CatKang Feb 19, 2017

Leviathan1995 Feb 19, 2017

CatKang Feb 20, 2017

CatKang Feb 19, 2017

Leviathan1995 Feb 19, 2017

KernelMaker commented Feb 19, 2017

Leviathan1995 commented Feb 19, 2017

CatKang Feb 20, 2017

CatKang Feb 20, 2017

Leviathan1995 Feb 20, 2017

CatKang Feb 20, 2017

CatKang Feb 20, 2017

CatKang Feb 20, 2017

CatKang commented Feb 20, 2017

CatKang commented Feb 21, 2017 •

edited

Loading

Support GEO #59

Support GEO #59

Conversation

Leviathan1995 commented Feb 15, 2017

CatKang commented Feb 16, 2017

Leviathan1995 commented Feb 16, 2017 • edited Loading

Leviathan1995 commented Feb 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KernelMaker commented Feb 19, 2017

Leviathan1995 commented Feb 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CatKang commented Feb 20, 2017

CatKang commented Feb 21, 2017 • edited Loading

Leviathan1995 commented Feb 16, 2017 •

edited

Loading

Leviathan1995 commented Feb 16, 2017 •

edited

Loading

CatKang commented Feb 21, 2017 •

edited

Loading