-
Notifications
You must be signed in to change notification settings - Fork 72
Test hang when set withThreadCount(1) and too many thread created in QueryBatcher #1279
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
Comments
Hi @ehennum, I asked Anu to help me take a look at this one. She has no idea either. Could you help me take a look at this one please? Thanks. |
@llinggit , I'm having trouble reproducing the issue. When I uncomment the But, I'm running with a single host. Does the cluster have to have multiple hosts to reproduce the issue? If so, that's provides a clue. |
Hi @ehennum, thanks for taking a look. I just installed the lasted nightly build and I cannot reproduce the hang in StringQueryHostBatcherTest.testProgressListener either. However, with one of other listed test, TestSplitters.testWriteOpsMultipleThreads, I can still reproduce the issue. I forgot which build I was, but you could try this one? |
Thanks, @llinggit . That reproduces the issue - 1 thread hangs but 2 threads succeeds. Investigating |
@llinggit , I believe I know what's going on:
I'm wondering whether the easiest and correct fix is to size the QueryTask queue to (threadCount * getDocToUriBatchRatio()) + 1. In principle, that should allow run() to queue all items and thus run to completion. What do you think? By the way, I think the test has a minor issue -- it would be better to call
instead of Thread.sleep(1000) |
@ehennum Thanks! Now I understand. Yes, that makes sense. Please go ahead with the fix. Thank you! |
The functional test was hanging before but ran green with this change. |
The previous commit bases the queue size on
with the reasoning that this ensures the QueryBatcher can specify the work to collect and process two batches of uris for each forest with latency in each thread. The queue could very likely be smaller, but a queue that's too large doesn't use a lot of memory and a queue that's too small causes a hang, so this change errs on the side of caution. |
Ran all the tests and they work fine. Thread count parameter was set to |
There's no server-side change in this issue. |
This is a corner case for task JAVA-195 and issue #1269.
For example in test StringQueryHostBatcherTest.testProgressListener, copied below, set "withThreadCount(1)" will cause a hang.
The text was updated successfully, but these errors were encountered: