-
Notifications
You must be signed in to change notification settings - Fork 160
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
[ISSUE #137] split TcpRemotingClient::m_ioService into m_dispatchService and m_handleService #156
Conversation
…nt::m_dispachService.
@@ -482,7 +498,7 @@ void TcpRemotingClient::ProcessData(const MemoryBlock& mem, const string& addr) | |||
LOG_DEBUG("find_response opaque:%d", opaque); | |||
processResponseCommand(pRespondCmd, pFuture); | |||
} else { | |||
processRequestCommand(pRespondCmd, addr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i suggest continue to use processRequestCommand, put m_handleService.post() to it, it will more clear to readers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. Do it will write another function for call processRequest and invokeOneway, it is unnecessary.
#if !defined(WIN32) && !defined(__APPLE__) | ||
string taskName = UtilAll::getProcessName(); | ||
prctl(PR_SET_NAME, "DispatchTP", 0, 0, 0); | ||
#endif | ||
for (int i = 0; i != m_dispatchThreadNum; ++i) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i < m_dispatchThreadNum is more safety than i!= m_dispatchThreadNum, it can handle the situation when some one change the i in {]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all codes for create threads used unequal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it not a good style of use unequal in for loop
prctl(PR_SET_NAME, taskName.c_str(), 0, 0, 0); | ||
#endif | ||
|
||
#if !defined(WIN32) && !defined(__APPLE__) | ||
prctl(PR_SET_NAME, "NetworkTP", 0, 0, 0); | ||
#endif | ||
for (int i = 0; i != m_pullThreadNum; ++i) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the same as pre suggestion
@@ -482,7 +498,7 @@ void TcpRemotingClient::ProcessData(const MemoryBlock& mem, const string& addr) | |||
LOG_DEBUG("find_response opaque:%d", opaque); | |||
processResponseCommand(pRespondCmd, pFuture); | |||
} else { | |||
processRequestCommand(pRespondCmd, addr); | |||
m_handleService.post(boost::bind(&TcpRemotingClient::processRequestCommand, this, pRespondCmd, addr)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when so many rebalance request come, m_handleService also be holded, so no thread to handle the response message?
@@ -482,7 +498,7 @@ void TcpRemotingClient::ProcessData(const MemoryBlock& mem, const string& addr) | |||
LOG_DEBUG("find_response opaque:%d", opaque); | |||
processResponseCommand(pRespondCmd, pFuture); | |||
} else { | |||
processRequestCommand(pRespondCmd, addr); | |||
m_handleService.post(boost::bind(&TcpRemotingClient::processRequestCommand, this, pRespondCmd, addr)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when so many rebalance request come(consumer changed), m_handleService also be holded, so no thread to handle the response message?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because rebalance is very quick without PullMsgEvent. we can do rebalance in an independent thread in future, after replace boost::asio with new implement. there is a RebalanceService in java, and i have written it in another branch for cpp.
What is the purpose of the change
use io-thread pool and work-thread pool in network callback to resolve deadlock in block-request.
Brief changelog
split TcpRemotingClient::m_ioService into m_dispatchService and m_handleService.
Verifying this change
XXXX
Follow this checklist to help us incorporate your contribution quickly and easily. Notice,
it would be helpful if you could finish the following 5 checklist(the last one is not necessary)before request the community to review your PR
.[ISSUE #123] Fix UnknownException when host config not exist
. Each commit in the pull request should have a meaningful subject line and body.