Skip to content

Shuwen.redis proxy#102

Merged
pengsven merged 11 commits intorenzhi_chashfrom
shuwen.redis_proxy
May 28, 2018
Merged

Shuwen.redis proxy#102
pengsven merged 11 commits intorenzhi_chashfrom
shuwen.redis_proxy

Conversation

@pengsven
Copy link
Contributor

  1. 封装1下resty.readis,添加:
  • 使用rpc_logging记录redis访问日志
  • 重试执行redis命令机制
  • 一个事务执行多条redis命令
  1. 添加客户端实现的一致性hash访问redis
  2. 添加代理实现的一致性hash访问redis

ps:测试后续补上

@pengsven pengsven requested review from drmingdrmer and liubaohai May 16, 2018 09:25
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4个返回...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

为啥不直接self:discard(), 为了某种风格一致吗?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

还是直接用self:discard()吧

@@ -0,0 +1,237 @@
local strutil = require("acid.strutil")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

感觉这个文件里大部分逻辑跟redis都没关系...

感觉这个文件实现了2个东西...一个是一致性hash,一个是nwr...感觉逻辑上是可以分开的, 并且独立于redis的.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1.acid.chash.lua已经是一个一致性hash的实现了,这里主要实现的是redis的一致性hash,
2.和nwr时有些耦合,我想想怎么分开

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个想完之后...有打算修改吗?还是就酱了?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nwr的区分开了,hash部分还是在redia_hash中

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个函数干嘛的...没懂...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

一个提供按照某种规则(如s2按照service_stat的server响应时间排序优先访问哪些server)优化chash选择的server接口,这里是一个直接返回addr是的默认函数

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HSET这个名字是copy过来的吧...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

一般对于一个业务,nwr是确定的, 读/写的wr必须 w+r>n才有意义.所以这里直接传1个参数包括nwr3个值, 对业务使用可能更友好(一个业务1个nwr配置) 实现也简单点. 不需要区分读写操作配置不同的nw / nr. 直接都传nwr进来. 好像这样好点?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这种配置方式是跟保海串通好的吧?~ <( ̄︶ ̄)>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我改了下,不需要get set hget hset,直接table的key lower()下就可以了

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(⊙v⊙)嗯


local redis_cmd_names = tableutil.keys(redis_cmd_model)

local function get_secret_key(access_key, secret_key)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个函数在干嘛...没懂啊...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aws_authenticator用到的,需要通过access_key得到secret_key

end

local function output(rst, err_code, err_msg)
local status, body, headers = 200, '', {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

output独立到一个公用函数里吧, 我记得acid里好多模块都有这么一坨定义

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

之前桐伟写过一个output(),没啥太大实用性 #78

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

为啥说没啥实用性?求详解

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_M.output(status, headers, body)只是实现了将发送出去status, headers, body

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

印象里..ptr是pointer的缩写...这里要缩写的话, ptn好点...

上面的注释是打酱油的吗...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

O(∩_∩)O哈哈~,一看你就是写C
ptn是pattern的缩写,比ptr些
想想用regex吧,ngx.re.match用的是regex
syntax: captures, err = ngx.re.match(subject, regex, options?, ctx?, res_table?)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

么看懂....

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

统一风格,加下ver吧

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这种调用方式不如xp提的self:exec()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我改了下,不需要get set hget hset,直接table的key lower()下就可以了


local redis_cli = acid_redis:new(ip, port, r_opts)

if pexpire ~= nil and cmd == 'hset' then

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

只有hset需要事务的方式吗,set呢

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set不需要事务,set命令本身支持带expire参数

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nwr作为一个table传进去,expire也传进去,直接self.redis_chash[cmd]()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里不太对吧,self有redis_cli吗,self.redis_chash?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里的cmd不需要配置,string.lower上面的cmd就可以了。少配置个参数。

@pengsven pengsven force-pushed the shuwen.redis_proxy branch from 30dd31b to d6ec951 Compare May 18, 2018 09:42
end

setmetatable(_M, {__index = function(_, cmd)
local method = function (self, ...)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

设置metatable有点不好懂, 直接列出所有的method逐个生成吧.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里是为了自动支持redis所有命令,不过从可读性来说是有点不太好懂,或者加个注释如何

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

静态设定吧,没有注释的代码最好了...

@@ -0,0 +1,237 @@
local strutil = require("acid.strutil")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个想完之后...有打算修改吗?还是就酱了?


local function get_chash(self)
local conf = self.conf
local now = ngx.time()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个update的代码好像跟async_cache的逻辑(或cache)模块一样,应该可以复用吧?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

async_cache好像可以

@pengsven pengsven force-pushed the shuwen.redis_proxy branch from d6ec951 to 9c82997 Compare May 23, 2018 11:38
@pengsven pengsven merged commit 267ef7a into renzhi_chash May 28, 2018
@pengsven pengsven deleted the shuwen.redis_proxy branch October 24, 2018 06:08
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

Successfully merging this pull request may close these issues.

3 participants

Comments