Keep-alive tests are faulty

[dmlloyd]

The tests for executor keep-alive are faulty. If they run on a slow system, then the jobs used will expire before or after they are expected to, causing invalid results.

Tests which depend on a sequence of concurrent events should use a strict enforcement of that sequencing of some sort. CountDownLatch is one choice but there are other options as well. Any test which depends on time-based behavior is faulty by design and will cause false negatives, which impacts development and release timelines.

Also, the diagnostic output mixes up "expected" and "actual" in some cases.

This issue was imported from JIRA. The original issue is JBTHR-67. It was originally reported by @dmlloyd.

[dmlloyd]

I agree with the general assessment of this issue, and I'm working on it. I can eliminate the chance for tasks being completed too soon or too late, and I can do the synchronization generally better. However, I don't think I'll be able to eliminate the time-based behaviour entirely. There are cases, where there needs to be countDownLatch.await(time, unit), and that will produce a false negative on a sufficiently slow machine. I don't know how to get rid of this, nor if it's possible to do that. Is that good enough?

---

Off-topic question:

Now that I'm playing with testThreadReuse, is there some sort of guarantee that when core threads are available, they will be used when a new task comes up, or is it a best effort approach? I have core size = 3, max size = 6, schedule three tasks and wait for them to complete. executor.getCompletedTaskCount returns 3 and executor.getActiveCount returns 0. When I schedule 3 more tasks, they usually reuse the core threads, but very rarely, a new one or two are created.

I know getCompletedTaskCount is an estimate, but I assumed it would never report a number greater than the actual completed task count. Am I looking at the wrong statistics? Or, perhaps, just maybe, have I found a bug? 😄

This comment was imported from JIRA. It was originally authored by @JittleJohnII.

[dmlloyd]

Even one false negative disrupts development & release so I'd prefer to have no possibility for it to happen in a normal test run. I will think about it some more.

On thread availability: if a thread does not become available (from the perspective of the job submitter) before the next task is submitted, the thread will not be reused. Unfortunately I don't think there's any way to determine that a thread has definitely become available to handle another task (which can be delayed infinitely in the pathological case), without actually monitoring its stack periodically or digging into its internal structure - and as a rule I won't maintain white-box tests because it's not practical in the face of code evolution.

That said, one option could be to add maintainable hooks or calls that could help examine the internal state of the pool a bit better for the purpose of testing.

The completed task count should never exceed the actual number of tasks completed. If it does, it's a bug!

This comment was imported from JIRA. It was originally authored by @dmlloyd.