REGR: Unsynchronized collection in a listener causing intermittent failure of unit test

[srinathgit]

The following unit test fails intermittently (it was run with latest code from 'develop' branch as of 5pm 12/2). It fails sometimes at assertEquals(numDocs, uris2.size()); or at assertEquals(numDocs, uris.size()); I am not sure if this is an issue with StructuredQueryDefinition but I did run 'TestStructuredQuery' in the functional tests and it ran fine all the times.

@Test
  public void test_B_InMemoryIterator() throws Exception {
    StructuredQueryBuilder sqb = new StructuredQueryBuilder();
    StructuredQueryDefinition query = sqb.value(sqb.jsonProperty("dept"), "HR");
    query.setCollections(qhbTestCollection);
    Set<String> uris = new HashSet<>();
    StringBuilder failures = new StringBuilder();
    QueryBatcher getUris = moveMgr.newQueryBatcher(query)
      .withThreadCount(6)
      .withBatchSize(5000)
      .onUrisReady( batch -> uris.addAll(Arrays.asList(batch.getItems())) )
      .onQueryFailure( throwable -> {
        throwable.printStackTrace();
        failures.append("ERROR:[" + throwable + "]\n");
      });
    moveMgr.startJob(getUris);
    getUris.awaitCompletion();
    if ( failures.length() > 0 ) fail(failures.toString());

    assertEquals(numDocs, uris.size());

    // now we have the uris, let's step through them
    AtomicInteger successDocs = new AtomicInteger();
    Set<String> uris2 = new HashSet<>();
    StringBuilder failures2 = new StringBuilder();
    QueryBatcher performDelete = moveMgr.newQueryBatcher(uris.iterator())
      .withThreadCount(2)
      .withBatchSize(99)
      .onUrisReady(new DeleteListener())
      .onUrisReady(batch -> successDocs.addAndGet(batch.getItems().length))
      .onUrisReady(batch -> uris2.addAll(Arrays.asList(batch.getItems())))
      .onQueryFailure( throwable -> {
        throwable.printStackTrace();
        failures2.append("ERROR:[" + throwable + "]\n");
      });
    moveMgr.startJob(performDelete);
    performDelete.awaitCompletion();

    if ( failures2.length() > 0 ) fail(failures2.toString());

    assertEquals(numDocs, uris2.size());
    assertEquals(numDocs, successDocs.get());

    SearchHandle results = client.newQueryManager().search(query, new SearchHandle());
    assertEquals(0, results.getTotalResults());
  }

Exception

java.lang.AssertionError: expected:<500> but was:<481>
	at org.junit.Assert.fail(Assert.java:88)
	at org.junit.Assert.failNotEquals(Assert.java:743)
	at org.junit.Assert.assertEquals(Assert.java:118)
	at org.junit.Assert.assertEquals(Assert.java:555)
	at org.junit.Assert.assertEquals(Assert.java:542)
	at com.marklogic.client.test.datamovement.QueryBatcherIteratorTest.test_B_InMemoryIterator(QueryBatcherIteratorTest.java:180)
	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 org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:271)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:70)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:238)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:63)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:236)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:53)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:229)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:309)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:86)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)

java.lang.AssertionError: expected:<500> but was:<495>
	at org.junit.Assert.fail(Assert.java:88)
	at org.junit.Assert.failNotEquals(Assert.java:743)
	at org.junit.Assert.assertEquals(Assert.java:118)
	at org.junit.Assert.assertEquals(Assert.java:555)
	at org.junit.Assert.assertEquals(Assert.java:542)
	at com.marklogic.client.test.datamovement.QueryBatcherIteratorTest.test_B_InMemoryIterator(QueryBatcherIteratorTest.java:159)
	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 org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:271)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:70)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:238)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:63)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:236)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:53)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:229)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:309)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:86)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)

[sammefford]

The cause of this failure is that I didn't synchronize the Set uris nor the Set uris2. Yet I popoulate those inside listeners which are run asynchronously (in multiple threads). I need to synchronize them so the writes are thread-safe. This is a good reminder that we need to tell our customers to avoid this same mistake when writing listeners. It's a safe assumption that even those who understand thread safety might forget and make this mistake from time to time. There's no way we can make a compiler catch this nor can we catch it. They're only going to discover their mistake after running their code many times and seeing intermittent failures like we did here.

[sammefford]

Ok, the fix is on develop branch