[
https://issues.apache.org/jira/browse/METRON-728?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15880905#comment-15880905
]
ASF GitHub Bot commented on METRON-728:
---------------------------------------
Github user mmiklavc commented on the issue:
https://github.com/apache/incubator-metron/pull/463
> We actually need it for all of the tests that ensure actual parallelism
and I've strengthened the other test impacted. We had gotten around it by being
too lenient, honestly. The rest of the test cases are non-parallel, so they are
correct as implemented.
> This is the correct fix, IMO. stream.parallel() does not give any
explicit guarantees about parallelism, just that it can parallelize batches
across a thread pool. This test essentially ensures that given enough tries, it
does parallelize.
Ok, I follow what's going on here and support the need to test for
instances where you may end up with the same thread by looping.
Just for some clarification, what is the reason that we are not leveraging
stream.isParallel() to check for parallelism? Would the concern here be that
because we're returning an object the implements the Stream interface, the
ReaderSpliterator could be changed to return something that does not abide by
the isParallel() contract?
> ReaderSpliteratorTest fails randomly and extremely rarely
> ---------------------------------------------------------
>
> Key: METRON-728
> URL: https://issues.apache.org/jira/browse/METRON-728
> Project: Metron
> Issue Type: Bug
> Affects Versions: 0.3.1
> Reporter: Justin Leet
> Assignee: Justin Leet
>
> See logs at
> https://travis-ci.org/justinleet/incubator-metron/builds/203298348
> I was able to reproduce this locally by calling
> {{testActuallyParallel_mediumBatch}} in a {{while(true)}} loop. It can also
> occur in {{testActuallyParallel_mediumBatchImplicitlyParallel()}}. I also
> had to add
> {{forkJoinPool.shutdownNow();}} to the end of the test, because otherwise OOM
> errors occur.
> My current assumption is that there's no guarantee you ever actually end up
> running in parallel, so in extremely rare cases you just end up running one
> thread.
> I've had it vary wildly when I hit it, from within a second or two to running
> for over a minute before an assertion failure occurs.
> We could just alter the assertion to be
> {code}Assert.assertTrue(threads.size() <= (int) Math.ceil(9.0 / 2) &&
> threads.size() >= 1);{code}
> This defeats the purpose of testing the parallelism a bit, but if there's no
> guarantee we actually get parallelism there's not a fantastic way to test it.
> Given the extreme rarity, we might want to just live with the fact that
> occasionally {{threads.size() >= 1}} hits the threads.size() == 1 case on the
> two tests.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
