Skip to content
This repository has been archived by the owner on Jun 23, 2022. It is now read-only.

nfs: remove nfs from service engine, refactor nfs to a single module #142

Merged
merged 1 commit into from
Aug 6, 2018

Conversation

shengofsun
Copy link
Contributor

No description provided.

@shengofsun shengofsun requested review from qinzuoyan and acelyc111 and removed request for LoveHeat August 1, 2018 07:37
qinzuoyan
qinzuoyan previously approved these changes Aug 1, 2018
@@ -84,7 +84,8 @@ void replica::on_checkpoint_timer()
(int64_t)_options->log_private_reserve_max_size_mb * 1024 * 1024,
(int64_t)_options->log_private_reserve_max_time_seconds);
if (status() == partition_status::PS_PRIMARY)
_counter_private_log_size->set(_private_log->total_size() / 1000000);
_counter_private_log_size->set(_private_log->total_size() /
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.

这个改回来了,我也没明白为啥原来换了一行。

}

aio_task_ptr nfs_node::copy_remote_directory(rpc_address remote,
const std::string &source_dir,
Copy link
Contributor

@neverchanje neverchanje Aug 2, 2018

Choose a reason for hiding this comment

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

接口传 String 参数推荐用 string_view

Copy link
Contributor Author

Choose a reason for hiding this comment

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

之前用的也是string,我就没改。string_view的好处是,传const char*进来,能避免一次拷贝吗?

Copy link
Contributor

@neverchanje neverchanje Aug 2, 2018

Choose a reason for hiding this comment

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

是的,行业内一般接口是用的 string_view,像 rocksdb 用的 Slice。内部类无所谓。这个我觉得尽量吧,不行也没关系。

}

aio_task_ptr nfs_node::copy_remote_files(rpc_address remote,
const std::string &source_dir,
Copy link
Contributor

Choose a reason for hiding this comment

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

我理解这个是我们 copy file 的新的接口,替代 dsn_file_copy_remote_files ?那应该用 string_view。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个因为有个vectorstd::string, 所以为了看起来一致舒服一些,就用了string了

@neverchanje
Copy link
Contributor

neverchanje commented Aug 2, 2018

几个问题哈:

  1. 为啥 nfs 原来要放在 service_node 里?
  2. remote_copy_response 这个我看 rdsn 现在也没有用到,我们只需要 remote_copy_request,不需要 response 吗?
  3. nfs_node_simple 这个 simple 是哪里 simple 了?我看也没有更复杂的实现?现在既然 nfs_node_simple 放在 nfs_node_impl.cpp 下,它的名字是不是叫 nfs_node_impl 更好。

@shengofsun
Copy link
Contributor Author

  1. 原来nfs在service_node里,是想在service_node层内置一个nfs的service, 所以以前也是有c接口的。但是这样让启动流程不清晰,并且让分层结构显示的不好。本质来看,nfs就是一个应用了rpc和disk_io这两个接口的一个普通service。
  2. remote_copy_request, 其实就是把copy_remote_files/directory这两个接口的输入参数整成一个struct, 这样传来传去方便一些。不会作为真正的request序列化出去,所以也不用response。序列化出去的req/resp定义在nfs.thrift里。
  3. simple的意思是“比较搓的一个实现”,在郭振宇的设想里,nfs应该有更牛逼的实现:splice/tee, 或者rdma。而不是用rpc封一个。这里就直接沿用原来的名称了。改名字可以后面改,一般重构代码,我会把尽量把整结构、挪动文件和重命名给拆开。这样便于review。

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants