JAVA-6071#1881
JAVA-6071#1881strogiyotec wants to merge 3 commits into
Conversation
hey @rozza , after looking at your listed fixes I didn't understand if they will fix NPE in ByteBufNIO but we can give it a try, I can remove my NPE fix but still keep longer timeout for connection pool to close , let me know if it works for you |
|
@strogiyotec apologies, my changes will help with the byte buffer management from I'd like to understand more about the failure scenario:
Looks like when a Is there anyway to replicate the error locally? I have two questions about the root cause:
|
JAVA-6071
This ticket involves two separate bugs in a test case
testBulkWriteHandlesWriteErrorsAcrossBatchesfor reactive driver , specifically whenorderedfield is falseIllegalState exception on Mono Timeout
The first part of the failed test is
Here is the link to the build
Here is the log that caught my attention
As you can see the time diff between two last logs is exactly two minutes , and according to our settings, the sync driver blocks mono by two minutes
So timeout happened and
Mono.blockthrewIllegalStateExceptionThis PR does not solve this issue, the
IllegalStateis expected, unless we want to override timeout for this test case only as it deals with a huge number of documents per batchConnection leak
Second asserting that failed on this test case is connection leak
Before looking at at the leak, I started checking what the test case is actually doing I noticed that it tries to insert documents in batches of 10K invalid(duplicate ids) docs per batch
The assertion that happens for connection leak happens here
The code is running a loop which fails after 2 seconds if not all connections are closed, in my local env I noticed that 2 seconds is not enough for such a huge number of documents to be processed so I introduced a new method that can propagate and override this timeout, after running this test case 100 times locally I didn't notice the connection leak anymore
To test it locally
IllegalStateExceptionRepeatedTestto this test case, in my case , I hardcodedordered=falseinstead of relying on parametrized test so that I can useRepeatedTestNullPointerException
The last part of this failed test is NullPointer here
The exception makes sense;
hasRemaining()throws an NPE because the underlying buffer becomes null once released. While I couldn't reproduce this after 100 local test runs, I added a guard forByteBufNIO. By usingasNio()first (which acts as a direct getter forByteBufNIObut avoids the buffer copying found in Netty or other implementations), we can safely check if the underlying buffer is null before checkinghasRemaining, for other implementation we still rely onhasRemainingI don't like this approach because of abstraction leak and open for any suggestions