rpc: remove uri resolver; move partition resolver to replication/client #205
Conversation
shengofsun
requested review from
qinzuoyan,
acelyc111,
hycdong,
mentoswang,
neverchanje,
vagetablechicken and
zhangyifan27
|
👍 |
|
zk老是下载失败，ci总是过不了，真惨…… |
|
内存我跑了下应该没问题。 |
shengofsun
force-pushed
the
refactor-resolver
branch
from
6862ea5
to
39d6b1f
Compare
|
issues in comments are fixed |
| const char *app_path); | ||
|
|
||
| template <typename TReq, typename TCallback> | ||
| dsn::rpc_response_task_ptr call_op(dsn::task_code code, |
There was a problem hiding this comment.
这个改名叫 call_replica 是不是合适一点。call_op 还不如简单叫 call。
There was a problem hiding this comment.
java那边叫operate，叫call_op是为了蹭那边的“operate”……
There was a problem hiding this comment.
我觉得 java 叫 operator 这个名字也很一般...
There was a problem hiding this comment.
我觉得叫op也挺好的啊，各种操作，不就是个op嘛……
There was a problem hiding this comment.
其实叫什么真的无所谓了。
不过我个人觉得呢，连同下面的call_task，都叫call得了（反正函数重载）。
就没这么多纠结了，哈哈哈...
|
我这边没啥其他问题，挺好的，后面就是要改 pegasus 的 rrdb client 了 👍 |
|
shengofsun/pegasus@b594e2f |
|
上面的comments你可以再看下 |
|
@neverchanje 函数名啥的就不改了吧，pegasus那边也得跟着改。 |
| static dsn::ref_ptr<partition_resolver> | ||
| get_resolver(const char *cluster_name, | ||
| const std::vector<dsn::rpc_address> &meta_list, | ||
| const char *app_path); |
There was a problem hiding this comment.
app_path 直接改为 app_name 吧。以前有uri，说path还情有可原，现在没有uri了，总觉得怪怪的，也不好理解。
| } | ||
|
|
||
| typedef partition_resolver *(*factory)(rpc_address &, const char *); | ||
| void call_task(const dsn::rpc_response_task_ptr &task); |
There was a problem hiding this comment.
建议对public的方法都增加注释，表名这个函数是做什么的，产生什么效果。
毕竟人家也是include头文件中的public方法嘛。
| { | ||
| if (req->header->gpid.value() != 0 && err != ERR_OK && err != ERR_HANDLER_NOT_FOUND && | ||
| err != ERR_APP_NOT_EXIST && err != ERR_OPERATION_DISABLED) { | ||
|
|
|
|
||
| rpc_response_task_ptr ctask = | ||
| dynamic_cast<rpc_response_task *>(task::get_current_task()); | ||
| partition_resolver *r = this; |
There was a problem hiding this comment.
这里是不是用 partition_resolver_ptr 好点？如果直接用裸指针， partition_resolver的生命周期在回调函数里面得不到保证，可能变成非法访问。
| const char *app_path); | ||
|
|
||
| template <typename TReq, typename TCallback> | ||
| dsn::rpc_response_task_ptr call_op(dsn::task_code code, |
There was a problem hiding this comment.
其实叫什么真的无所谓了。
不过我个人觉得呢，连同下面的call_task，都叫call得了（反正函数重载）。
就没这么多纠结了，哈哈哈...
大家可以仔细看看这个pr，比如说客户端的内存泄露什么的。
我自己机器上在跑killtest，目前看貌似内存还挺平稳。明天我追加一下跑一晚上的内存测试结果。
这个pr的内容有：
好处：