[GitHub] [arrow] zhztheplayer commented on a diff in pull request #11182: ARROW-14034: [Java] Unexpected Allocator states created after allocating buffer whose AllocationManager has different size from the requested size

[GitBox <gi...@apache.org>]

zhztheplayer commented on code in PR #11182:
URL: https://github.com/apache/arrow/pull/11182#discussion_r883232645


##########
java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java:
##########
@@ -438,6 +441,94 @@ public ArrowBuf empty() {
         }).build());
   }
 
+  @Test
+  public void testLargerGrantedSize() {
+    // actual size is larger than request size
+    final BaseAllocator allocator = createAllocatorWithFixedSizeAllocationManager(1024, MAX_ALLOCATION);
+    final ArrowBuf arrowBuf = allocator.buffer(16L);
+    assertEquals(1024L, arrowBuf.getPossibleMemoryConsumed());
+    assertEquals(1024L, arrowBuf.getActualMemoryConsumed());
+    assertEquals(1024L, allocator.getAllocatedMemory());
+    assertDoesNotThrow(() -> AutoCloseables.close(arrowBuf, allocator));
+  }
+
+  @Test
+  public void testLargerGrantedSizeOverLimit() {
+    // actual size is larger than request size, and is beyond allocation limit
+    final BaseAllocator allocator = createAllocatorWithFixedSizeAllocationManager(1024, 1023);
+    assertThrows(OutOfMemoryException.class, () -> {
+      allocator.buffer(16L); // should throw exception
+    });
+    assertEquals(0L, allocator.getAllocatedMemory());
+    assertDoesNotThrow(() -> AutoCloseables.close(allocator));
+  }
+
+  @Test
+  public void testDifferentGrantedSizeTransfer() {
+    // actual size is different from request size, then transfer balance.
+    final BaseAllocator root = createAllocatorWithFixedSizeAllocationManager(1024, MAX_ALLOCATION);
+    final BufferAllocator c1 = root.newChildAllocator("child1", 0, MAX_ALLOCATION);
+    final BufferAllocator c2 = root.newChildAllocator("child2", 0, MAX_ALLOCATION);
+    final ArrowBuf arrowBuf = c1.buffer(16L);
+    assertEquals(1024L, arrowBuf.getPossibleMemoryConsumed());
+    assertEquals(1024L, arrowBuf.getActualMemoryConsumed());
+    assertEquals(1024L, c1.getAllocatedMemory());
+    assertEquals(1024L, root.getAllocatedMemory());
+    OwnershipTransferResult r = arrowBuf.getReferenceManager().transferOwnership(arrowBuf, c2);
+    assertTrue(r.getAllocationFit());
+    final ArrowBuf transferredBuffer = r.getTransferredBuffer();
+    assertEquals(1024L, arrowBuf.getPossibleMemoryConsumed());
+    assertEquals(0L, arrowBuf.getActualMemoryConsumed());
+    assertEquals(0L, c1.getAllocatedMemory());
+    assertEquals(1024L, c2.getAllocatedMemory());
+    assertEquals(1024L, root.getAllocatedMemory());
+    assertDoesNotThrow(() -> AutoCloseables.close(arrowBuf, transferredBuffer, c1, c2, root));
+  }
+
+  @Test
+  public void testSmallerGrantedSize() {
+    // actual size is larger than request size
+    final BaseAllocator allocator = createAllocatorWithFixedSizeAllocationManager(1, MAX_ALLOCATION);
+    assertThrows(UnsupportedOperationException.class, () -> allocator.buffer(16L));

Review Comment:
   The comment `// actual size is larger than request size` was wrong here. I've corrected it.
   
   Java doesn't seem to provide a built-in `IllegalOperationException` definition. Would you think it is worth to create one?



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org