You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kafka.apache.org by ij...@apache.org on 2018/10/21 00:41:46 UTC

[kafka] branch 2.1 updated: MINOR: Less restrictive assertion in flaky BufferPool test (#5799)

This is an automated email from the ASF dual-hosted git repository.

ijuma pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/kafka.git


The following commit(s) were added to refs/heads/2.1 by this push:
     new 3d581b6  MINOR: Less restrictive assertion in flaky BufferPool test (#5799)
3d581b6 is described below

commit 3d581b61daa0ccf0fb2678ef130f5fe6af6e7057
Author: Colin Hicks <co...@gmail.com>
AuthorDate: Sat Oct 20 20:40:44 2018 -0400

    MINOR: Less restrictive assertion in flaky BufferPool test (#5799)
    
    Decrease the lower bound for expected available memory, as thread
    scheduling entails that a variable amount of deallocation happens by
    the point of assertion.
    
    Also make minor clarifications to test logic and comments.
    
    The passing rate improved from 98% to 100% locally after these
    changes (100+ runs).
    
    Reviewers: Ismael Juma <is...@juma.me.uk>
    
    ### Committer Checklist (excluded from commit message)
    - [ ] Verify design and implementation
    - [ ] Verify test coverage and CI build status
    - [ ] Verify documentation (including upgrade notes)
---
 .../clients/producer/internals/BufferPoolTest.java      | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/clients/src/test/java/org/apache/kafka/clients/producer/internals/BufferPoolTest.java b/clients/src/test/java/org/apache/kafka/clients/producer/internals/BufferPoolTest.java
index ce74eb1..8e44fa1 100644
--- a/clients/src/test/java/org/apache/kafka/clients/producer/internals/BufferPoolTest.java
+++ b/clients/src/test/java/org/apache/kafka/clients/producer/internals/BufferPoolTest.java
@@ -154,7 +154,7 @@ public class BufferPoolTest {
 
     /**
      * Test if Timeout exception is thrown when there is not enough memory to allocate and the elapsed time is greater than the max specified block time.
-     * And verify that the allocation should finish soon after the maxBlockTimeMs.
+     * And verify that the allocation attempt finishes soon after the maxBlockTimeMs.
      */
     @Test
     public void testBlockTimeout() throws Exception {
@@ -162,10 +162,10 @@ public class BufferPoolTest {
         ByteBuffer buffer1 = pool.allocate(1, maxBlockTimeMs);
         ByteBuffer buffer2 = pool.allocate(1, maxBlockTimeMs);
         ByteBuffer buffer3 = pool.allocate(1, maxBlockTimeMs);
-        // First two buffers will be de-allocated within maxBlockTimeMs since the most recent de-allocation
+        // The first two buffers will be de-allocated within maxBlockTimeMs since the most recent allocation
         delayedDeallocate(pool, buffer1, maxBlockTimeMs / 2);
         delayedDeallocate(pool, buffer2, maxBlockTimeMs);
-        // The third buffer will be de-allocated after maxBlockTimeMs since the most recent de-allocation
+        // The third buffer will be de-allocated after maxBlockTimeMs since the most recent allocation
         delayedDeallocate(pool, buffer3, maxBlockTimeMs / 2 * 5);
 
         long beginTimeMs = Time.SYSTEM.milliseconds();
@@ -175,9 +175,11 @@ public class BufferPoolTest {
         } catch (TimeoutException e) {
             // this is good
         }
-        assertTrue("available memory" + pool.availableMemory(), pool.availableMemory() >= 9 && pool.availableMemory() <= 10);
-        long endTimeMs = Time.SYSTEM.milliseconds();
-        assertTrue("Allocation should finish not much later than maxBlockTimeMs", endTimeMs - beginTimeMs < maxBlockTimeMs + 1000);
+        // Thread scheduling sometimes means that deallocation varies by this point
+        assertTrue("available memory " + pool.availableMemory(), pool.availableMemory() >= 8 && pool.availableMemory() <= 10);
+        long durationMs = Time.SYSTEM.milliseconds() - beginTimeMs;
+        assertTrue("TimeoutException should not throw before maxBlockTimeMs", durationMs >= maxBlockTimeMs);
+        assertTrue("TimeoutException should throw soon after maxBlockTimeMs", durationMs < maxBlockTimeMs + 1000);
     }
 
     /**
@@ -193,7 +195,8 @@ public class BufferPoolTest {
         } catch (TimeoutException e) {
             // this is good
         }
-        assertTrue(pool.queued() == 0);
+        assertEquals(0, pool.queued());
+        assertEquals(1, pool.availableMemory());
     }
 
     /**