-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add HttpClient pool #2187
base: master
Are you sure you want to change the base?
Add HttpClient pool #2187
Conversation
poolPtr->wait(); | ||
poolPtr.reset(); | ||
}; | ||
(func(args), ...); |
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 thread pool may not created by HttpClientPool, we should not quit thread pool unconditionally.
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.
update
dispatchPool_ = std::make_shared<trantor::EventLoopThreadPool>(1); | ||
dispatchPool_->start(); | ||
} | ||
loopPtr_ = dispatchPool_->getNextLoop(); |
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.
What does dispatchPool_ do? I only see one loop use here.
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.
update
@hwc0919 @marty1885 please check |
@nqf Sorry Things cam up and I'm very busy at work RN. Will check sometime this week, most likely the weekends. From an initial look, I've concerns of this interacting unfavorably with our HTTP/2 client (you shouldn't pool HTTP/2). |
yes, this is a client connection pool for http1.1, and i think http2 should not need it. http2 has not been merged into master yet |
2fc63f6
to
0229064
Compare
No description provided.