Fix sendrecv port bind#9595
Conversation
gongweibao
left a comment
gongweibao
left a comment
There was a problem hiding this comment.
Just small comments.
Thanks!
| VLOG(3) << "shutdown, do not TryToRegisterNewSendOne"; | ||
| return; | ||
| } | ||
| while (scope_ == nullptr) { |
There was a problem hiding this comment.
These should be deleted.
|
|
||
| framework::Executor executor(dev_place); | ||
|
|
||
| // FIXME(Yancey1989): initialize rpc server with lazy mode. |
There was a problem hiding this comment.
This line is not useful.
|
|
||
| void RunServer(std::shared_ptr<detail::AsyncGRPCServer> service); | ||
|
|
||
| static void CreateTensorFromMessageType(framework::Variable *var, |
There was a problem hiding this comment.
Should we put functions' definition in header files?
typhoonzero
force-pushed
the
fix_test_sendrecv_portbind
branch
from
cd09715 to
00f8e63
Compare
… fix_test_sendrecv_portbind
wangkuiyi
left a comment
wangkuiyi
left a comment
There was a problem hiding this comment.
Thanks for this PR!
I have a look-irrelevant comment but could be important.
| set_source_files_properties(send_vars_op.cc PROPERTIES COMPILE_FLAGS ${DISTRIBUTE_COMPILE_FLAGS}) | ||
| op_library(send_barrier_op DEPS ${DISTRIBUTE_DEPS}) | ||
| set_source_files_properties(send_barrier_op.cc PROPERTIES COMPILE_FLAGS ${DISTRIBUTE_COMPILE_FLAGS}) | ||
| set_source_files_properties(send_recv_op_test.cc PROPERTIES COMPILE_FLAGS ${DISTRIBUTE_COMPILE_FLAGS}) |
There was a problem hiding this comment.
Please remove the use of set_source_files_properties as discussed in #9612
There was a problem hiding this comment.
Because of grpc implementation, we have to use this when linking with grpc, and this can not be present at generic.cmake.
typhoonzero
dismissed
wangkuiyi’s stale review
Need to merge this fix first, will discuss in the issue
typhoonzero
force-pushed
the
fix_test_sendrecv_portbind
branch
from
6cd9f1e to
a9cc9ed
Compare
… fix_test_sendrecv_portbind
typhoonzero
force-pushed
the
fix_test_sendrecv_portbind
branch
from
a9cc9ed to
b03fa88
Compare
Yancey0623
left a comment
Yancey0623
left a comment
There was a problem hiding this comment.
LGTM, we could merge this fix firstly to fix the unit test error.
4 participants
Fix unit test
send_recv_op_test.ccerror binding used port.Fix grpc_server use an uninitialized scope.