Skip to content
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

[tests] Make BrokerClientIntegrationTest testing behavior deterministic #2585

Merged
merged 3 commits into from Sep 21, 2018

Conversation

@sijie
Copy link
Contributor

sijie commented Sep 14, 2018

Motivation

The test is flaky.

2018-09-14\T\17:46:29.848 [ERROR] testUnsupportedBatchMessageConsumer(org.apache.pulsar.client.impl.BrokerClientIntegrationTest)  Time elapsed: 3.161 s  <<< FAILURE!
java.lang.AssertionError: Received message my-message-5 did not match the expected message my-message-0 expected [my-message-0] but found [my-message-5]
        at org.testng.Assert.fail(Assert.java:96)
        at org.testng.Assert.failNotEquals(Assert.java:776)
        at org.testng.Assert.assertEqualsImpl(Assert.java:137)
        at org.testng.Assert.assertEquals(Assert.java:118)
        at org.apache.pulsar.client.api.ProducerConsumerBase.testMessageOrderAndDuplicates(ProducerConsumerBase.java:51)
        at org.apache.pulsar.client.impl.BrokerClientIntegrationTest.testUnsupportedBatchMessageConsumer(BrokerClientIntegrationTest.java:357)
        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.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:124)
        at org.testng.internal.InvokeMethodRunnable.runOne(InvokeMethodRunnable.java:54)
        at org.testng.internal.InvokeMethodRunnable.run(InvokeMethodRunnable.java:44)
        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:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at java.lang.Thread.run(Thread.java:748)

The problem is due to we used a time based batching policy in testing. There is no guarantee how messages can be batched, hence
the ordering and duplication check can fail on shared subscription

Changes

Set batching delay to a very large value and make sure the messages are in one batch.

+ .g/COMMIT_EDITMSG
[tests] Make BrokerClientIntegrationTest testing behavior deterministic

*Motivation*

The test is flaky.

```
2018-09-14\T\17:46:29.848 [ERROR] testUnsupportedBatchMessageConsumer(org.apache.pulsar.client.impl.BrokerClientIntegrationTest)  Time elapsed: 3.161 s  <<< FAILURE!
java.lang.AssertionError: Received message my-message-5 did not match the expected message my-message-0 expected [my-message-0] but found [my-message-5]
        at org.testng.Assert.fail(Assert.java:96)
        at org.testng.Assert.failNotEquals(Assert.java:776)
        at org.testng.Assert.assertEqualsImpl(Assert.java:137)
        at org.testng.Assert.assertEquals(Assert.java:118)
        at org.apache.pulsar.client.api.ProducerConsumerBase.testMessageOrderAndDuplicates(ProducerConsumerBase.java:51)
        at org.apache.pulsar.client.impl.BrokerClientIntegrationTest.testUnsupportedBatchMessageConsumer(BrokerClientIntegrationTest.java:357)
        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.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:124)
        at org.testng.internal.InvokeMethodRunnable.runOne(InvokeMethodRunnable.java:54)
        at org.testng.internal.InvokeMethodRunnable.run(InvokeMethodRunnable.java:44)
        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:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at java.lang.Thread.run(Thread.java:748)
```

The problem is due to we used a `time` based batching policy in testing. There is no guarantee how messages can be batched, hence
the ordering and duplication check can fail on shared subscription

*Changes*

Set batching delay to a very large value and make sure the messages are in one batch.

@sijie sijie self-assigned this Sep 14, 2018

@sijie sijie requested review from ivankelly, merlimat, srkukarni and rdhabalia Sep 14, 2018

@sijie

This comment has been minimized.

Copy link
Contributor Author

sijie commented Sep 14, 2018

run java8 tests

@sijie

This comment has been minimized.

Copy link
Contributor Author

sijie commented Sep 15, 2018

retest this please

String message = "my-message-" + i;
batchProducer.sendAsync(message.getBytes());
lastSendFuture = batchProducer.sendAsync(message.getBytes());
}

This comment has been minimized.

Copy link
@ivankelly

ivankelly Sep 17, 2018

Contributor

you could use producer.flush() here.

This comment has been minimized.

Copy link
@sijie

sijie Sep 17, 2018

Author Contributor

changed to use producer.flush

@sijie

This comment has been minimized.

Copy link
Contributor Author

sijie commented Sep 17, 2018

retest this please

@sijie

This comment has been minimized.

Copy link
Contributor Author

sijie commented Sep 18, 2018

run java8 tests

@sijie

This comment has been minimized.

Copy link
Contributor Author

sijie commented Sep 19, 2018

run java8 tests

3 similar comments
@sijie

This comment has been minimized.

Copy link
Contributor Author

sijie commented Sep 19, 2018

run java8 tests

@sijie

This comment has been minimized.

Copy link
Contributor Author

sijie commented Sep 20, 2018

run java8 tests

@sijie

This comment has been minimized.

Copy link
Contributor Author

sijie commented Sep 21, 2018

run java8 tests

@ivankelly

This comment has been minimized.

Copy link
Contributor

ivankelly commented Sep 21, 2018

rerun integration tests
rerun java8 tests

@sijie sijie added this to the 2.2.0 milestone Sep 21, 2018

@sijie sijie merged commit 15f4587 into apache:master Sep 21, 2018

2 of 3 checks passed

Jenkins: Integration Tests FAILURE
Details
Jenkins: C++ / Python Tests SUCCESS
Details
Jenkins: Java 8 - Unit Tests SUCCESS
Details

@sijie sijie deleted the sijie:fix_broker_client_integration_test branch Sep 21, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.