checking potential fix for build failures on AsyncQueryForwardingServletTest #1233
Conversation
|
ok, this fixes the build. I don't know whether to wait for original bug to get fixed or put this in the RouterJettyServerInitializer.java also. (this bug might cause router to just block if jetty server is not initialized with enough threads) what do you think? |
|
Adding a thread limit without a way to record the usage seems risky. Is it possible to add the number of used threads as a metric or something? |
|
Also, can you add a unit test so behavior can be defined for what happens if the thread count limit is met? This might require making the limit configurable. |
|
In the short term, if there's a way to make this as big or larger than the servlet executor pool would at least provide some limit compared to prior behavior. |
|
@himanshug thanks for tracking this down! I wonder if this affects the router as well or not. I'm a bit puzzled as to why this test started failing, since we didn't change any of that code or updated jetty. Something on the Travis end must have changed that triggers this bug, maybe the default number of threads is tied to the number of cores, and that may have changed. |
|
@himanshug do you mind adding a comment above the line you added to refer to the bug? I'll merge after that. |
|
As a general FYI, it looks like this behavior is still present in 9.3.0 but in a different place than referenced in the original ticket. |
himanshug
force-pushed
the
async_query_forwarding_servlet_test_fix
branch
from
00bd45d
to
b5b32db
Compare
|
@xvrl added the comment @drcrallen I believe your comments are towards the conclusion that we should update RouterJettyServerInitializer.java with configurable number of threads and not necessarity wait from jetty to fix the issue? |
…t_test_fix checking potential fix for build failures on AsyncQueryForwardingServletTest
|
@himanshug : yes, especially since the workaround is pretty straight forward. |
in test explicitly setting maxThreads=256 for ProxyServlet to work around https://tickets.puppetlabs.com/browse/TK-152
if this gets fixed, then we might want to do this in RouterJettyServerInitializer.java also