From 72d6d85f2eeb9dc822b9062cbd70daf256a0aad5 Mon Sep 17 00:00:00 2001 From: Denys Kuzmenko Date: Tue, 7 Jul 2026 12:37:41 +0300 Subject: [PATCH 1/3] =?UTF-8?q?HIVE-29703:=20LLAP:=20Parquet=20footer=20la?= =?UTF-8?q?rger=20than=20alloc.max=20fails=20to=20cache=20=E2=80=94=20Runt?= =?UTF-8?q?imeException:=20Trying=20to=20allocate=20N;=20max=20is=20M?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../hive/llap/io/metadata/MetadataCache.java | 56 ++++---- ...adataCache.java => TestMetadataCache.java} | 124 +++++++++++------- 2 files changed, 103 insertions(+), 77 deletions(-) rename llap-server/src/test/org/apache/hadoop/hive/llap/cache/{TestOrcMetadataCache.java => TestMetadataCache.java} (83%) diff --git a/llap-server/src/java/org/apache/hadoop/hive/llap/io/metadata/MetadataCache.java b/llap-server/src/java/org/apache/hadoop/hive/llap/io/metadata/MetadataCache.java index 8be5a06631eb..7554a301b417 100644 --- a/llap-server/src/java/org/apache/hadoop/hive/llap/io/metadata/MetadataCache.java +++ b/llap-server/src/java/org/apache/hadoop/hive/llap/io/metadata/MetadataCache.java @@ -257,39 +257,43 @@ public LlapBufferOrBuffers putFileMetadata(Object fileKey, int length, InputStre } - @SuppressWarnings({ "rawtypes", "unchecked" }) + @SuppressWarnings("unchecked") private LlapBufferOrBuffers wrapBbForFile(LlapBufferOrBuffers result, Object fileKey, int length, InputStream stream, CacheTag tag, AtomicBoolean isStopped) throws IOException { - if (result != null) return result; + if (result != null) { + return result; + } int maxAlloc = allocator.getMaxAllocation(); - LlapMetadataBuffer[] largeBuffers = null; - if (maxAlloc < length) { - largeBuffers = new LlapMetadataBuffer[length / maxAlloc]; - for (int i = 0; i < largeBuffers.length; ++i) { - largeBuffers[i] = new LlapMetadataBuffer<>(fileKey, tag); - } - allocator.allocateMultiple(largeBuffers, maxAlloc, null, isStopped); - for (int i = 0; i < largeBuffers.length; ++i) { - readIntoCacheBuffer(stream, maxAlloc, largeBuffers[i]); - } + if (length <= maxAlloc) { + // The whole footer fits in a single buffer - the overwhelmingly common case. + LlapMetadataBuffer buffer = new LlapMetadataBuffer<>(fileKey, tag); + allocator.allocateMultiple(new MemoryBuffer[] { buffer }, length, null, isStopped); + readIntoCacheBuffer(stream, length, buffer); + return buffer; + } + // Larger footers are split across maxAlloc-sized chunks, the last one holding the remainder. + LlapMetadataBuffer[] largeBuffers = new LlapMetadataBuffer[length / maxAlloc]; + for (int i = 0; i < largeBuffers.length; ++i) { + largeBuffers[i] = new LlapMetadataBuffer<>(fileKey, tag); + } + allocator.allocateMultiple(largeBuffers, maxAlloc, null, isStopped); + for (int i = 0; i < largeBuffers.length; ++i) { + readIntoCacheBuffer(stream, maxAlloc, largeBuffers[i]); } int smallSize = length % maxAlloc; if (smallSize == 0) { - return new LlapMetadataBuffers(largeBuffers); - } else { - LlapMetadataBuffer[] smallBuffer = new LlapMetadataBuffer[1]; - smallBuffer[0] = new LlapMetadataBuffer(fileKey, tag); - allocator.allocateMultiple(smallBuffer, length, null, isStopped); - readIntoCacheBuffer(stream, smallSize, smallBuffer[0]); - if (largeBuffers == null) { - return smallBuffer[0]; // This is the overwhelmingly common case. - } else { - LlapMetadataBuffer[] cacheData = new LlapMetadataBuffer[largeBuffers.length + 1]; - System.arraycopy(largeBuffers, 0, cacheData, 0, largeBuffers.length); - cacheData[largeBuffers.length] = smallBuffer[0]; - return new LlapMetadataBuffers<>(cacheData); - } + return new LlapMetadataBuffers<>(largeBuffers); } + // Allocate the remainder only; the last chunk is smaller than maxAlloc. + LlapMetadataBuffer smallBuffer = new LlapMetadataBuffer<>(fileKey, tag); + allocator.allocateMultiple(new MemoryBuffer[] { smallBuffer }, smallSize, null, isStopped); + readIntoCacheBuffer(stream, smallSize, smallBuffer); + + LlapMetadataBuffer[] cacheData = new LlapMetadataBuffer[largeBuffers.length + 1]; + System.arraycopy(largeBuffers, 0, cacheData, 0, largeBuffers.length); + cacheData[largeBuffers.length] = smallBuffer; + + return new LlapMetadataBuffers<>(cacheData); } private static void readIntoCacheBuffer( diff --git a/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestOrcMetadataCache.java b/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestMetadataCache.java similarity index 83% rename from llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestOrcMetadataCache.java rename to llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestMetadataCache.java index 5ec15beccfb9..ee90d74af8e7 100644 --- a/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestOrcMetadataCache.java +++ b/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestMetadataCache.java @@ -20,6 +20,7 @@ import static org.apache.hadoop.hive.llap.cache.LlapCacheableBuffer.INVALIDATE_OK; import static org.junit.Assert.*; +import java.io.ByteArrayInputStream; import java.io.IOException; import java.nio.ByteBuffer; import java.util.Random; @@ -51,7 +52,9 @@ import org.junit.Assert; import org.junit.Test; -public class TestOrcMetadataCache { +public class TestMetadataCache { + private static final int MAX_ALLOC = 64; + private static class DummyCachePolicy implements LowLevelCachePolicy { int lockCount = 0, unlockCount = 0; @@ -115,12 +118,7 @@ public void updateMaxSize(long maxSize) { @Test public void testCaseSomePartialBuffersAreEvicted() { - final DummyMemoryManager mm = new DummyMemoryManager(); - final DummyCachePolicy cp = new DummyCachePolicy(); - final int MAX_ALLOC = 64; - final LlapDaemonCacheMetrics metrics = LlapDaemonCacheMetrics.create("", ""); - final BuddyAllocator alloc = new BuddyAllocator(false, false, 8, MAX_ALLOC, 1, 4096, 0, null, mm, metrics, null, true); - final MetadataCache cache = new MetadataCache(alloc, mm, cp, true, metrics); + final MetadataCache cache = newMetadataCache(); final Object fileKey1 = new Object(); final Random rdm = new Random(); final ByteBuffer smallBuffer = ByteBuffer.allocate(2 * MAX_ALLOC); @@ -138,14 +136,8 @@ public void testCaseSomePartialBuffersAreEvicted() { } @Test - public void testBuffers() throws Exception { - DummyMemoryManager mm = new DummyMemoryManager(); - DummyCachePolicy cp = new DummyCachePolicy(); - final int MAX_ALLOC = 64; - LlapDaemonCacheMetrics metrics = LlapDaemonCacheMetrics.create("", ""); - BuddyAllocator alloc = new BuddyAllocator( - false, false, 8, MAX_ALLOC, 1, 4096, 0, null, mm, metrics, null, true); - MetadataCache cache = new MetadataCache(alloc, mm, cp, true, metrics); + public void testBuffers() { + MetadataCache cache = newMetadataCache(); Object fileKey1 = new Object(); Random rdm = new Random(); @@ -185,6 +177,65 @@ public void testBuffers() throws Exception { assertFalse(b0.incRef() > 0); // Should have also been thrown out. } + /** + * A footer of exactly maxAlloc: the split condition used a strict {@code <}, so no buffer was + * allocated and the InputStream put path returned an empty wrapper (NPE on lock). + */ + @Test + public void testStreamFooterEqualToMaxAlloc() throws Exception { + verifyFooterFromStream(newMetadataCache(), MAX_ALLOC, true); + } + + /** + * A footer larger than maxAlloc is split into a multi-buffer wrapper: an exact multiple of maxAlloc + * (only full chunks) and a multiple plus a remainder (full chunks + a smaller last chunk). The + * remainder buffer used to be allocated with the full footer length, exceeding the max allocation. + */ + @Test + public void testStreamFooterLargerThanMaxAlloc() throws Exception { + MetadataCache cache = newMetadataCache(); + verifyFooterFromStream(cache, 2 * MAX_ALLOC, false); + verifyFooterFromStream(cache, 2 * MAX_ALLOC + 3, false); + } + + private MetadataCache newMetadataCache() { + return newMetadataCache(4096); + } + + private MetadataCache newMetadataCache(int arenaSize) { + return newMetadataCache(arenaSize, new DummyCachePolicy()); + } + + private MetadataCache newMetadataCache(int arenaSize, DummyCachePolicy cp) { + DummyMemoryManager mm = new DummyMemoryManager(); + LlapDaemonCacheMetrics metrics = LlapDaemonCacheMetrics.create("", ""); + BuddyAllocator alloc = new BuddyAllocator( + false, false, 8, MAX_ALLOC, 1, arenaSize, 0, null, mm, metrics, null, true); + return new MetadataCache(alloc, mm, cp, true, metrics); + } + + private void verifyFooterFromStream(MetadataCache cache, int length, boolean expectSingleBuffer) + throws IOException { + Object fileKey = new Object(); + ByteBuffer footer = ByteBuffer.allocate(length); + new Random().nextBytes(footer.array()); + + LlapBufferOrBuffers result = cache.putFileMetadata( + fileKey, length, new ByteArrayInputStream(footer.array()), null, null); + cache.decRefBuffer(result); + assertEquals(expectSingleBuffer, result.getSingleLlapBuffer() != null); + assertEquals(footer, reassemble(result)); + + result = cache.getFileMetadata(fileKey); + assertEquals(footer, reassemble(result)); + cache.decRefBuffer(result); + } + + private ByteBuffer reassemble(LlapBufferOrBuffers result) { + LlapAllocatorBuffer single = result.getSingleLlapBuffer(); + return single != null ? single.getByteBufferDup() : extractResultBbs(result); + } + public ByteBuffer extractResultBbs(LlapBufferOrBuffers result) { int totalLen = 0; for (LlapAllocatorBuffer buf : result.getMultipleLlapBuffers()) { @@ -199,14 +250,9 @@ public ByteBuffer extractResultBbs(LlapBufferOrBuffers result) { } @Test - public void testIncompleteCbs() throws Exception { - DummyMemoryManager mm = new DummyMemoryManager(); + public void testIncompleteCbs() { DummyCachePolicy cp = new DummyCachePolicy(); - final int MAX_ALLOC = 64; - LlapDaemonCacheMetrics metrics = LlapDaemonCacheMetrics.create("", ""); - BuddyAllocator alloc = new BuddyAllocator( - false, false, 8, MAX_ALLOC, 1, 4096, 0, null, mm, metrics, null, true); - MetadataCache cache = new MetadataCache(alloc, mm, cp, true, metrics); + MetadataCache cache = newMetadataCache(4096, cp); DataCache.BooleanRef gotAllData = new DataCache.BooleanRef(); Object fileKey1 = new Object(); @@ -239,13 +285,7 @@ public void testIncompleteCbs() throws Exception { @Test public void testGetOrcTailForPath() throws Exception { - DummyMemoryManager mm = new DummyMemoryManager(); - DummyCachePolicy cp = new DummyCachePolicy(); - final int MAX_ALLOC = 64; - LlapDaemonCacheMetrics metrics = LlapDaemonCacheMetrics.create("", ""); - BuddyAllocator alloc = new BuddyAllocator( - false, false, 8, MAX_ALLOC, 1, 4 * 4096, 0, null, mm, metrics, null, true); - MetadataCache cache = new MetadataCache(alloc, mm, cp, true, metrics); + MetadataCache cache = newMetadataCache(4 * 4096); Path path = new Path("../data/files/alltypesorc"); Configuration jobConf = new Configuration(); @@ -260,13 +300,7 @@ public void testGetOrcTailForPath() throws Exception { @Test public void testGetOrcTailForPathWithFileId() throws Exception { - DummyMemoryManager mm = new DummyMemoryManager(); - DummyCachePolicy cp = new DummyCachePolicy(); - final int MAX_ALLOC = 64; - LlapDaemonCacheMetrics metrics = LlapDaemonCacheMetrics.create("", ""); - BuddyAllocator alloc = new BuddyAllocator( - false, false, 8, MAX_ALLOC, 1, 4 * 4096, 0, null, mm, metrics, null, true); - MetadataCache cache = new MetadataCache(alloc, mm, cp, true, metrics); + MetadataCache cache = newMetadataCache(4 * 4096); Path path = new Path("../data/files/alltypesorc"); Configuration jobConf = new Configuration(); @@ -284,13 +318,7 @@ public void testGetOrcTailForPathWithFileId() throws Exception { @Test public void testGetOrcTailForPathWithFileIdChange() throws Exception { - DummyMemoryManager mm = new DummyMemoryManager(); - DummyCachePolicy cp = new DummyCachePolicy(); - final int MAX_ALLOC = 64; - LlapDaemonCacheMetrics metrics = LlapDaemonCacheMetrics.create("", ""); - BuddyAllocator alloc = new BuddyAllocator( - false, false, 8, MAX_ALLOC, 1, 4 * 4096, 0, null, mm, metrics, null, true); - MetadataCache cache = new MetadataCache(alloc, mm, cp, true, metrics); + MetadataCache cache = newMetadataCache(4 * 4096); Path path = new Path("../data/files/alltypesorc"); Configuration jobConf = new Configuration(); @@ -317,14 +345,8 @@ public void testGetOrcTailForPathCacheNotReady() throws Exception { } @Test - public void testProactiveEvictionMark() throws Exception { - DummyMemoryManager mm = new DummyMemoryManager(); - DummyCachePolicy cp = new DummyCachePolicy(); - final int MAX_ALLOC = 64; - LlapDaemonCacheMetrics metrics = LlapDaemonCacheMetrics.create("", ""); - BuddyAllocator alloc = new BuddyAllocator( - false, false, 8, MAX_ALLOC, 1, 4096, 0, null, mm, metrics, null, true); - MetadataCache cache = new MetadataCache(alloc, mm, cp, true, metrics); + public void testProactiveEvictionMark() { + MetadataCache cache = newMetadataCache(); long fn1 = 1; long fn2 = 2; From 3bf78175de8f83c6d9f255ae4ffda546a04273a7 Mon Sep 17 00:00:00 2001 From: Denys Kuzmenko Date: Wed, 8 Jul 2026 15:06:56 +0300 Subject: [PATCH 2/3] review comments #1 --- .../hive/llap/io/metadata/MetadataCache.java | 74 ++++++++++++++----- .../hive/llap/cache/TestMetadataCache.java | 47 ++++++++++++ 2 files changed, 104 insertions(+), 17 deletions(-) diff --git a/llap-server/src/java/org/apache/hadoop/hive/llap/io/metadata/MetadataCache.java b/llap-server/src/java/org/apache/hadoop/hive/llap/io/metadata/MetadataCache.java index 7554a301b417..c287beaf6582 100644 --- a/llap-server/src/java/org/apache/hadoop/hive/llap/io/metadata/MetadataCache.java +++ b/llap-server/src/java/org/apache/hadoop/hive/llap/io/metadata/MetadataCache.java @@ -264,36 +264,76 @@ private LlapBufferOrBuffers wrapBbForFile(LlapBufferOrBuffers result, return result; } int maxAlloc = allocator.getMaxAllocation(); + // Buffers are locked and handed to the cache only after this returns, so release whatever we + // allocated if a later read or allocation throws - otherwise it leaks (nothing else reclaims it). if (length <= maxAlloc) { // The whole footer fits in a single buffer - the overwhelmingly common case. LlapMetadataBuffer buffer = new LlapMetadataBuffer<>(fileKey, tag); allocator.allocateMultiple(new MemoryBuffer[] { buffer }, length, null, isStopped); - readIntoCacheBuffer(stream, length, buffer); - return buffer; + boolean done = false; + try { + readIntoCacheBuffer(stream, length, buffer); + done = true; + return buffer; + } finally { + if (!done) { + deallocateQuietly(buffer); + } + } } // Larger footers are split across maxAlloc-sized chunks, the last one holding the remainder. LlapMetadataBuffer[] largeBuffers = new LlapMetadataBuffer[length / maxAlloc]; for (int i = 0; i < largeBuffers.length; ++i) { largeBuffers[i] = new LlapMetadataBuffer<>(fileKey, tag); } + // allocateMultiple is all-or-nothing: on success every chunk is allocated; on failure it + // releases whatever it reserved, so a throw here needs no cleanup from us. allocator.allocateMultiple(largeBuffers, maxAlloc, null, isStopped); - for (int i = 0; i < largeBuffers.length; ++i) { - readIntoCacheBuffer(stream, maxAlloc, largeBuffers[i]); - } - int smallSize = length % maxAlloc; - if (smallSize == 0) { - return new LlapMetadataBuffers<>(largeBuffers); + LlapMetadataBuffer smallBuffer = null; + boolean done = false; + try { + for (int i = 0; i < largeBuffers.length; ++i) { + readIntoCacheBuffer(stream, maxAlloc, largeBuffers[i]); + } + int smallSize = length % maxAlloc; + if (smallSize == 0) { + done = true; + return new LlapMetadataBuffers<>(largeBuffers); + } + // Allocate the remainder only; the last chunk is smaller than maxAlloc. + LlapMetadataBuffer remainder = new LlapMetadataBuffer<>(fileKey, tag); + allocator.allocateMultiple(new MemoryBuffer[] { remainder }, smallSize, null, isStopped); + smallBuffer = remainder; // Registered for cleanup only now that allocation succeeded. + readIntoCacheBuffer(stream, smallSize, remainder); + + LlapMetadataBuffer[] cacheData = new LlapMetadataBuffer[largeBuffers.length + 1]; + System.arraycopy(largeBuffers, 0, cacheData, 0, largeBuffers.length); + cacheData[largeBuffers.length] = remainder; + + done = true; + return new LlapMetadataBuffers<>(cacheData); + } finally { + if (!done) { + for (LlapMetadataBuffer buffer : largeBuffers) { + deallocateQuietly(buffer); + } + if (smallBuffer != null) { + deallocateQuietly(smallBuffer); + } + } } - // Allocate the remainder only; the last chunk is smaller than maxAlloc. - LlapMetadataBuffer smallBuffer = new LlapMetadataBuffer<>(fileKey, tag); - allocator.allocateMultiple(new MemoryBuffer[] { smallBuffer }, smallSize, null, isStopped); - readIntoCacheBuffer(stream, smallSize, smallBuffer); - - LlapMetadataBuffer[] cacheData = new LlapMetadataBuffer[largeBuffers.length + 1]; - System.arraycopy(largeBuffers, 0, cacheData, 0, largeBuffers.length); - cacheData[largeBuffers.length] = smallBuffer; + } - return new LlapMetadataBuffers<>(cacheData); + /** + * Releases a footer buffer that was allocated but not yet published to the cache. The buffer must + * be unlocked (refCount 0, as deallocate asserts); callers here run before it is ever locked. + */ + private void deallocateQuietly(MemoryBuffer buffer) { + try { + allocator.deallocate(buffer); + } catch (Throwable t) { + LlapIoImpl.LOG.error("Ignoring the cleanup error after another error", t); + } } private static void readIntoCacheBuffer( diff --git a/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestMetadataCache.java b/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestMetadataCache.java index ee90d74af8e7..04f939a26e1c 100644 --- a/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestMetadataCache.java +++ b/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestMetadataCache.java @@ -35,6 +35,7 @@ import org.apache.hadoop.hive.common.io.DataCache; import org.apache.hadoop.hive.common.io.DiskRange; import org.apache.hadoop.hive.common.io.DiskRangeList; +import org.apache.hadoop.hive.common.io.encoded.MemoryBuffer; import org.apache.hadoop.hive.conf.HiveConf; import org.apache.hadoop.hive.llap.IllegalCacheConfigurationException; @@ -97,10 +98,12 @@ public void debugDumpShort(StringBuilder sb) { private static class DummyMemoryManager implements MemoryManager { private int allocs; + private long reservedBytes; @Override public void reserveMemory(long memoryToReserve, AtomicBoolean isStopped) { ++allocs; + reservedBytes += memoryToReserve; } @Override public long evictMemory(long memoryToEvict) { @@ -109,11 +112,16 @@ public void reserveMemory(long memoryToReserve, AtomicBoolean isStopped) { @Override public void releaseMemory(long memUsage) { + reservedBytes -= memUsage; } @Override public void updateMaxSize(long maxSize) { } + + long reservedBytes() { + return reservedBytes; + } } @Test @@ -198,6 +206,45 @@ public void testStreamFooterLargerThanMaxAlloc() throws Exception { verifyFooterFromStream(cache, 2 * MAX_ALLOC + 3, false); } + /** + * If reading the footer stream fails after some buffers were already allocated, every allocated + * buffer must be released back to the allocator. Otherwise the long-lived LLAP daemon leaks arena + * memory on every failed footer read (a truncated file, an IO error, or an oversized chunk). + */ + @Test + public void testStreamFooterReadFailureReleasesBuffers() { + assertFooterReadFailureReleasesBuffers(MAX_ALLOC); // single-buffer path + assertFooterReadFailureReleasesBuffers(2 * MAX_ALLOC); // multi-buffer, exact multiple + assertFooterReadFailureReleasesBuffers(2 * MAX_ALLOC + 3); // multi-buffer, with remainder + } + + private void assertFooterReadFailureReleasesBuffers(int length) { + final int arenaSize = 4096; + DummyMemoryManager mm = new DummyMemoryManager(); + LlapDaemonCacheMetrics metrics = LlapDaemonCacheMetrics.create("", ""); + BuddyAllocator alloc = new BuddyAllocator( + false, false, 8, MAX_ALLOC, 1, arenaSize, 0, null, mm, metrics, null, true); + MetadataCache cache = new MetadataCache(alloc, mm, new DummyCachePolicy(), true, metrics); + + Object fileKey = new Object(); + // One byte short of the footer, so reading the last chunk hits EOF part-way through. + ByteArrayInputStream truncated = new ByteArrayInputStream(new byte[length - 1]); + try { + cache.putFileMetadata(fileKey, length, truncated, null, null); + fail("Expected an EOFException for a truncated footer stream of length " + length); + } catch (IOException expected) { + // expected + } + assertEquals("Allocated buffers must be released after a failed footer read (length " + length + ")", + 0, mm.reservedBytes()); + assertNull("A failed put must not leave a cache entry", cache.getFileMetadata(fileKey)); + + // Independent check against the real allocator: leaked buffers keep FLAG_NEW_ALLOC and can never + // be force-discarded, so re-filling the whole arena succeeds only if nothing leaked. + MemoryBuffer[] wholeArena = new MemoryBuffer[arenaSize / MAX_ALLOC]; + alloc.allocateMultiple(wholeArena, MAX_ALLOC); + } + private MetadataCache newMetadataCache() { return newMetadataCache(4096); } From 99a1bad193a46d8824fca59cb6a5acccb1ac78f0 Mon Sep 17 00:00:00 2001 From: Denys Kuzmenko Date: Tue, 14 Jul 2026 16:13:23 +0300 Subject: [PATCH 3/3] review comments #2 --- .../apache/hadoop/hive/llap/io/metadata/MetadataCache.java | 3 ++- .../apache/hadoop/hive/llap/cache/TestMetadataCache.java | 7 ++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/llap-server/src/java/org/apache/hadoop/hive/llap/io/metadata/MetadataCache.java b/llap-server/src/java/org/apache/hadoop/hive/llap/io/metadata/MetadataCache.java index c287beaf6582..184c8e1d8c8c 100644 --- a/llap-server/src/java/org/apache/hadoop/hive/llap/io/metadata/MetadataCache.java +++ b/llap-server/src/java/org/apache/hadoop/hive/llap/io/metadata/MetadataCache.java @@ -287,8 +287,9 @@ private LlapBufferOrBuffers wrapBbForFile(LlapBufferOrBuffers result, largeBuffers[i] = new LlapMetadataBuffer<>(fileKey, tag); } // allocateMultiple is all-or-nothing: on success every chunk is allocated; on failure it - // releases whatever it reserved, so a throw here needs no cleanup from us. + // releases whatever it reserved. allocator.allocateMultiple(largeBuffers, maxAlloc, null, isStopped); + LlapMetadataBuffer smallBuffer = null; boolean done = false; try { diff --git a/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestMetadataCache.java b/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestMetadataCache.java index 04f939a26e1c..369f0968baef 100644 --- a/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestMetadataCache.java +++ b/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestMetadataCache.java @@ -21,6 +21,7 @@ import static org.junit.Assert.*; import java.io.ByteArrayInputStream; +import java.io.EOFException; import java.io.IOException; import java.nio.ByteBuffer; import java.util.Random; @@ -212,13 +213,13 @@ public void testStreamFooterLargerThanMaxAlloc() throws Exception { * memory on every failed footer read (a truncated file, an IO error, or an oversized chunk). */ @Test - public void testStreamFooterReadFailureReleasesBuffers() { + public void testStreamFooterReadFailureReleasesBuffers() throws IOException { assertFooterReadFailureReleasesBuffers(MAX_ALLOC); // single-buffer path assertFooterReadFailureReleasesBuffers(2 * MAX_ALLOC); // multi-buffer, exact multiple assertFooterReadFailureReleasesBuffers(2 * MAX_ALLOC + 3); // multi-buffer, with remainder } - private void assertFooterReadFailureReleasesBuffers(int length) { + private void assertFooterReadFailureReleasesBuffers(int length) throws IOException { final int arenaSize = 4096; DummyMemoryManager mm = new DummyMemoryManager(); LlapDaemonCacheMetrics metrics = LlapDaemonCacheMetrics.create("", ""); @@ -232,7 +233,7 @@ private void assertFooterReadFailureReleasesBuffers(int length) { try { cache.putFileMetadata(fileKey, length, truncated, null, null); fail("Expected an EOFException for a truncated footer stream of length " + length); - } catch (IOException expected) { + } catch (EOFException expected) { // expected } assertEquals("Allocated buffers must be released after a failed footer read (length " + length + ")",