[GitHub] [arrow] lwhite1 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>]

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


##########
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:
   Since the error is caused by a request for an amount of memory that is larger than the allocated buffer, it might be more appropriate to throw IllegalOperationException than UnsupportedOperation, which indicates that the requested operation is never legal, as when adding an object to an ImmutableList. 



-- 
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