-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Adding jetty HTTP client queue size metric #17100
base: master
Are you sure you want to change the base?
Conversation
@@ -133,6 +133,7 @@ Most metric values reset each emission period, as specified in `druid.monitoring | |||
|`jetty/threadPool/min`|Number of minimum threads allocatable.|`druid.server.http.numThreads` plus a small fixed number of threads allocated for Jetty acceptors and selectors.| | |||
|`jetty/threadPool/max`|Number of maximum threads allocatable.|`druid.server.http.numThreads` plus a small fixed number of threads allocated for Jetty acceptors and selectors.| | |||
|`jetty/threadPool/queueSize`|Size of the worker queue.|Not much higher than `druid.server.http.queueSize`.| | |||
|`jetty/httpClient/threadpool/queueSize`|Size of the worker queue at jetty http client.|Less than or equal to 1024(default value).| |
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.
How do you intend to use this metric?
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.
This is to monitor if we have a sufficient number of Connections per destination. Let's suppose the latency is high at the broker. It might be possible the request was waiting at the jetty client-side queue, causing the latency to go high.
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 jetty client module is only being used on coordinator and router though. And it's unusual that you are hitting a limit 1024.
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 service did you see this log line in
`jetty/httpClient/threadpool/queueSize` Size of the worker queue at jetty http client. Less than or equal to 1024(default value).
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.
we do see this logline from the router:
Max requests queued per destination 1024 exceeded for HttpDestination[$(broker)]@4b9ee947,queue=1024,pool=DuplexConnectionPool@7698916c[c=0/100/100,a=100,i=0,q=1024]
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 jetty client module is only being used on coordinator and router though.
ohh, But the broker also communicates with data nodes, so what does the broker use for communicating with data nodes?
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.
Actually, I am more interested in the broker-side client queue. Can you help me with which client we use there?
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.
public class HttpClientModule implements Module |
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.
@abhishekagarwal87, we are not setting the thread pool executor in the HttpClientModule. In the jetty server module, we also expose queue size from the thread pool; so is there any other way we can emit queue size here in HttpClientModule?
Fixes #XXXX.
Description
Fixed the bug ...
Renamed the class ...
Added a forbidden-apis entry ...
Release note
Key changed/added classes in this PR
MyFoo
OurBar
TheirBaz
This PR has: