-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Switching RPC library in new pserver implement #2602
Comments
The current master client does not use SWIG but directly import .so build with cgo. |
有两条调用路径,一个是swig,这个目前其实绕不开,因为是trainer(cpp) <---> pserver(go) ,第二条路目前是cgo实现的.so,目前master -- client实际上就是做了一些rpc调用,所以如果直接有好用的python binding,client端就不需要写go了,想起来确实不错 |
trainer(cpp) <---> pserver(go) 这条路径也可以简化,由于grpc是有c++ binding的,所以可以直接用c++来调go的pserver接口,而不是先用cgo实现一个c的client,然后再调用。 |
相当于换一个有多重语言binding的rpc框架,这样各个组件用自己的语言实现就行了,还可以随意切换实现的语言,赞 |
Just follow the related issue link, I found that here is a very close idea rpc issue
|
Thanks for the great suggestion! It's a very good point that we have a complicated code path due mixing C, C++, Python, Go together, we need to write glue code for language bindings. I would love to have our code reduced (fewer human written code fewer error). This is a great idea. Using grpc which automatically generates language bindings, we can remove the glue code for RPC written by humans, replacing it with grpc's native Python code (as illustrated in the second graph from your comment). In essence, Go in the client lib is removed, Python calls the remote server directly with grpc. However, we need to consider this problem: The Go code in the client is not only If we take a look at the master client code here it calls If we take a look at the pserver client code here and here, it has the logic to partition parameters into different pservers, and the logic to ask etcd if the current trainer will be the one trainer to init all parameters on pservers. From the two paragraphs above, we can see that there are chunks of application logic in Go in the client, which grpc can not replace. So if we want to remove Go in the client, we will need to write these logics again in Python. I am not fully convinced to do these logics in Python for the following reasons:
I totally agree that we need the system to be as simple as possible, and it would awesome if we have a feasible solution reduce this complication. Would love to know your thoughts. |
In my former think, if we use grpc, we can use it's client in native language such as cpp in pserver client and python in master client, but just like @helinwang 's comments, it's certainly a problem if we have written some library in go and want to use it in both server and client such as RecordIO. For lib like RecordIO, It seems we always need a glue and can not totally replace it with rpc language binding. |
@jacquesqiao if implement pserver and master client in c++, there's no good choice for etcd client, so we have to call the etcd For pserver client, if using gRPC, it should be implemented using c++ so that the For master client, we can implement using python(recordio have python binding), so it will be simpler, but the client is so small that it will bring not buch benifit at all. So after some thinking, I would prefer to continue using the current way. |
Related issue: #1721
We are using
net/rpc
in our implementation currently. In fact, I found that when we writing a client for trainers to call from, we have to implement the c binding first, and then compile it to a shared object(so), and then use swig to export that library, and then using ctypes to import so lib from python. The procedure can be displayed below:As we can see, we need to implement so many things. The root cause is that
net/rpc
in go don't have c or python bindings, we have to convert go-written clients to c and then to python.I would like to introduce a more simple and efficient way using
grpc
instead ofnet/rpc
as below:If we all switch to grpc and using python binding to implement the pserver client and master client, it will be much more simpler.
Some etcd3 python client bindings:
The text was updated successfully, but these errors were encountered: