You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/08/23 22:45:26 UTC
[GitHub] [pulsar] rdhabalia opened a new pull request #11753: [pulsar-test] enable pooled netty allocator for unit-tests
rdhabalia opened a new pull request #11753:
URL: https://github.com/apache/pulsar/pull/11753
### Motivation
It fixes #11750 which has flaky test that fails because test checks if consumed payload is allocated in direct-memory and it doesn't give a guarantee to allocate message in direct-memory as default allocator is not pooled and therefore, sometime client accepts messages in non-pooled (heap-memory) allocator and test fails to validate message buffer type.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari merged pull request #11753: [pulsar-test] enable pooled netty allocator for unit-tests
Posted by GitBox <gi...@apache.org>.
lhotari merged pull request #11753:
URL: https://github.com/apache/pulsar/pull/11753
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] rdhabalia commented on pull request #11753: [pulsar-test] enable pooled netty allocator for unit-tests
Posted by GitBox <gi...@apache.org>.
rdhabalia commented on pull request #11753:
URL: https://github.com/apache/pulsar/pull/11753#issuecomment-904847109
> I hope that this change won't introduce instability due to other leaks in the tests
I agree. one of the issue with `-Dpulsar.allocator.pooled=true` is test starts leaking due to test-failure or missed to close/cleanup broker/client then we will see memory failure soon. So, I was thinking to revert back `-Dpulsar.allocator.pooled=false` and keep direct-memory check in if condition but before that this PR is merged now. let me create a PR and we can merge it if that makes sense.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #11753: [pulsar-test] enable pooled netty allocator for unit-tests
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #11753:
URL: https://github.com/apache/pulsar/pull/11753#issuecomment-904344206
It looks like there's a test which isn't compatible with pooled allocator. This test is failing:
```
Error: Tests run: 7, Failures: 1, Errors: 0, Skipped: 6, Time elapsed: 4.129 s <<< FAILURE! - in org.apache.pulsar.broker.service.TransactionMarkerDeleteTest
Error: testMarkerDelete(org.apache.pulsar.broker.service.TransactionMarkerDeleteTest) Time elapsed: 0.018 s <<< FAILURE!
java.lang.UnsupportedOperationException: direct buffer
at io.netty.buffer.PooledUnsafeDirectByteBuf.array(PooledUnsafeDirectByteBuf.java:226)
at org.apache.pulsar.broker.service.TransactionMarkerDeleteTest.testMarkerDelete(TransactionMarkerDeleteTest.java:122)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:566)
at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132)
at org.testng.internal.InvokeMethodRunnable.runOne(InvokeMethodRunnable.java:45)
at org.testng.internal.InvokeMethodRunnable.call(InvokeMethodRunnable.java:73)
at org.testng.internal.InvokeMethodRunnable.call(InvokeMethodRunnable.java:11)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
at java.base/java.lang.Thread.run(Thread.java:829)
```
It can be fixed by changing [line 108 in TransactionMarkerDeleteTest](https://github.com/apache/pulsar/blob/3fcfb22e71c9774f710d659030d4af01eb05c956/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/TransactionMarkerDeleteTest.java#L108) from `ByteBuf payload = PooledByteBufAllocator.DEFAULT.buffer(0);` to `ByteBuf payload = PooledByteBufAllocator.DEFAULT.heapBuffer(0);`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #11753: [pulsar-test] enable pooled netty allocator for unit-tests
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #11753:
URL: https://github.com/apache/pulsar/pull/11753#issuecomment-904410525
I pushed the fix to TransactionMarkerDeleteTest to this PR since it would be nice to get this PR merged asap to improve CI stability.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #11753: [pulsar-test] enable pooled netty allocator for unit-tests
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #11753:
URL: https://github.com/apache/pulsar/pull/11753#issuecomment-951627304
@rdhabalia:Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari edited a comment on pull request #11753: [pulsar-test] enable pooled netty allocator for unit-tests
Posted by GitBox <gi...@apache.org>.
lhotari edited a comment on pull request #11753:
URL: https://github.com/apache/pulsar/pull/11753#issuecomment-904344206
It looks like there's a test which isn't compatible with pooled allocator. This test is failing:
```
Error: Tests run: 7, Failures: 1, Errors: 0, Skipped: 6, Time elapsed: 4.129 s <<< FAILURE! - in org.apache.pulsar.broker.service.TransactionMarkerDeleteTest
Error: testMarkerDelete(org.apache.pulsar.broker.service.TransactionMarkerDeleteTest) Time elapsed: 0.018 s <<< FAILURE!
java.lang.UnsupportedOperationException: direct buffer
at io.netty.buffer.PooledUnsafeDirectByteBuf.array(PooledUnsafeDirectByteBuf.java:226)
at org.apache.pulsar.broker.service.TransactionMarkerDeleteTest.testMarkerDelete(TransactionMarkerDeleteTest.java:122)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:566)
at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132)
at org.testng.internal.InvokeMethodRunnable.runOne(InvokeMethodRunnable.java:45)
at org.testng.internal.InvokeMethodRunnable.call(InvokeMethodRunnable.java:73)
at org.testng.internal.InvokeMethodRunnable.call(InvokeMethodRunnable.java:11)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
at java.base/java.lang.Thread.run(Thread.java:829)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #11753: [pulsar-test] enable pooled netty allocator for unit-tests
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #11753:
URL: https://github.com/apache/pulsar/pull/11753#issuecomment-951625989
@rdhabalia:Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org