[
https://issues.apache.org/jira/browse/OOZIE-2550?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15321776#comment-15321776
]
Robert Kanter commented on OOZIE-2550:
--------------------------------------
I think the {{AtomicReferenceArray}} is a bit overkill here. The two threads
should get non-overlapping sets of IDs, which are used as indexes. So there
should never be any race conditions on the individual array elements
themselves. If that somehow does happen, then we'll know because the test will
fail because at least one of the array elements will be {{false}}. In fact, it
would be helpful if this test does fail, if we were to log the IDs generated by
each thread.
So, I think we should:
# Replace the {{AtomicReferenceArray}} with a boolean array:
{code:java}
boolean[] result = new boolean[size];
{code}
This should default to {{false}} for all elements, but if you want to be extra
sure you can always add {{Arrays.fill(result, false);}}
# Print out the IDs generated by each thread and which thread generated them
for easier debugging.
# In {{setUp()}}, use a try-finally block to make sure the uuid service there
is always destroyed.
#- Actually, I think we'd be better off making {{setMaxSequence}} a static
method. It's setting a static variable, so that makes sense. Then you don't
have to create a new {{ZKUUIDService}} here.
# I might be missing something here, but don't we need to reset the sequence to
start at 0 somewhere? The array is from 0 to 10000, but what if a previous
test had left the sequence at, say 50? We use the IDs as indexes, so this
wouldn't work correctly, right?
> Flaky tests in TestZKUUIDService.java
> -------------------------------------
>
> Key: OOZIE-2550
> URL: https://issues.apache.org/jira/browse/OOZIE-2550
> Project: Oozie
> Issue Type: Bug
> Components: tests
> Reporter: Peter Bacsko
> Assignee: Peter Bacsko
> Priority: Minor
> Attachments: OOZIE-2550-001.patch, OOZIE-2550-002.patch,
> OOZIE-2550-003.patch, OOZIE-2550-004.patch
>
>
> 1. Test case testMultipleIDGeneration_withMultiThread in TestZKUUIDService
> uses an ArrayList which is written by two threads simultaneously. This is
> dangerous and the list must be externally synchronized to prevent race
> conditions.
> Another problem is that you cannot put items at arbitrary indexes in an
> ArrayList. For example, if the list is empty, the following code throws
> ArrayIndexOutOfBoundException:
> {code}
> List<Boolean> test = new ArrayList<>(10000);
> test.add(22, true);
> {code}
> In an unlucky scheduling event, the following can happen:
> 1. The list has a certain number of elements, the value of "size" inside the
> list is 1000
> 2. Thread-1 retrieves the next ID from ZK UUID service, which is 1001
> 3. Thread-2 retrieves the next ID from ZK UUID service, which is 1002
> 4. Thread-2 runs faster than Thread-1 and tries to call {{list.add(1002,
> true)}} which fails.
> The following error was caught during a test run:
> {code}
> Tests run: 8, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 53.571 sec
> <<< FAILURE!
> testMultipleIDGeneration_withMultiThread(org.apache.oozie.service.TestZKUUIDService)
> Time elapsed: 0.02 sec <<< ERROR!
> java.lang.IndexOutOfBoundsException: Index: 89, Size: 89
> at java.util.ArrayList.rangeCheck(ArrayList.java:653)
> at java.util.ArrayList.get(ArrayList.java:429)
> at
> org.apache.oozie.service.TestZKUUIDService.testMultipleIDGeneration_withMultiThread(TestZKUUIDService.java:134)
> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> at
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
> at
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.lang.reflect.Method.invoke(Method.java:498)
> at junit.framework.TestCase.runTest(TestCase.java:168)
> at junit.framework.TestCase.runBare(TestCase.java:134)
> at junit.framework.TestResult$1.protect(TestResult.java:110)
> at junit.framework.TestResult.runProtected(TestResult.java:128)
> at junit.framework.TestResult.run(TestResult.java:113)
> at junit.framework.TestCase.run(TestCase.java:124)
> at junit.framework.TestSuite.runTest(TestSuite.java:243)
> at junit.framework.TestSuite.run(TestSuite.java:238)
> at
> org.junit.internal.runners.JUnit38ClassRunner.run(JUnit38ClassRunner.java:83)
> at org.junit.runners.Suite.runChild(Suite.java:128)
> at org.junit.runners.Suite.runChild(Suite.java:24)
> at org.junit.runners.ParentRunner$3.run(ParentRunner.java:231)
> at
> java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
> at java.util.concurrent.FutureTask.run(FutureTask.java:266)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:745)
> {code}
> 2. The order in which the tests are executed is random. The problem is that
> testResetSequence sets the maximum sequence number to 900. Because this value
> is stored in a static variable inside ZKUUIDService, it affects
> testIDGeneration and testMultipleIDGeneration if these tests are run after
> testResetSequence.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
