diff --git a/persistit/core/src/main/java/com/persistit/BufferPool.java b/persistit/core/src/main/java/com/persistit/BufferPool.java index 51167ff54..2fdaa333a 100644 --- a/persistit/core/src/main/java/com/persistit/BufferPool.java +++ b/persistit/core/src/main/java/com/persistit/BufferPool.java @@ -741,12 +741,13 @@ Buffer get(final Volume vol, final long page, final boolean writer, final boolea * throwing an InUseException * @return Buffer The Buffer describing the buffer containing the page. * @throws InUseException - * if the specific lock could not be acquired within the - * specified timeout + * if the specific lock could not be acquired or no buffer + * could be allocated for the page within the specified timeout */ Buffer get(final Volume vol, final long page, final boolean writer, final boolean wantRead, final long timeout) throws PersistitException { final int hash = hashIndex(vol, page); + final long start = System.currentTimeMillis(); Buffer buffer = null; for (;;) { @@ -782,34 +783,52 @@ Buffer get(final Volume vol, final long page, final boolean writer, final boolea // in the page from the Volume. // buffer = allocBuffer(); - Debug.$assert1.t(!buffer.isDirty()); - Debug.$assert0.t(buffer != _hashTable[hash]); - Debug.$assert0.t(buffer.getNext() != buffer); - - buffer.setPageAddressAndVolume(page, vol); - buffer.setNext(_hashTable[hash]); - _hashTable[hash] = buffer; - // - // It's not really valid yet, but it does have a writer - // claim on it so no other Thread can access it. In the - // meantime, any other Thread seeking access to the same - // page will find it. - // - buffer.setValid(); - if (vol.isTemporary() || vol.isLockVolume()) { - buffer.setTemporary(); - } else { - buffer.clearTemporary(); + if (buffer != null) { + Debug.$assert1.t(!buffer.isDirty()); + Debug.$assert0.t(buffer != _hashTable[hash]); + Debug.$assert0.t(buffer.getNext() != buffer); + + buffer.setPageAddressAndVolume(page, vol); + buffer.setNext(_hashTable[hash]); + _hashTable[hash] = buffer; + // + // It's not really valid yet, but it does have a writer + // claim on it so no other Thread can access it. In the + // meantime, any other Thread seeking access to the same + // page will find it. + // + buffer.setValid(); + if (vol.isTemporary() || vol.isLockVolume()) { + buffer.setTemporary(); + } else { + buffer.clearTemporary(); + } + Debug.$assert0.t(buffer.getNext() != buffer); } - Debug.$assert0.t(buffer.getNext() != buffer); } } finally { _hashLocks[hash % HASH_LOCKS].unlock(); } + if (buffer == null) { + /* + * allocBuffer() swept the whole pool without finding an + * evictable buffer. Wait for one to be released and retry the + * search until the timeout budget expires. The wait must + * happen here, after the finally block above has released the + * hash lock, so that no thread sleeps while holding a lock + * stripe. + */ + if (System.currentTimeMillis() - start >= timeout) { + throw new InUseException("Thread " + Thread.currentThread().getName() + + " failed to allocate a buffer for page " + page + " in " + vol + " within " + timeout + + " ms"); + } + Util.sleep(RETRY_SLEEP_TIME); + continue; + } if (mustClaim) { boolean claimed = false; boolean same = true; - final long start = System.currentTimeMillis(); while (same && !claimed && System.currentTimeMillis() - start < timeout) { /* * We're here because we found the page we want, but another @@ -963,8 +982,6 @@ public Buffer getBufferCopy(final int index) throws IllegalArgumentException { * currently available. The buffer has a writer claim. * @throws PersistitException * if a persistence error occurs while evicting a page - * @throws IllegalStateException - * if there is no available buffer. */ private Buffer allocBuffer() throws PersistitException { @@ -1070,7 +1087,7 @@ private Buffer allocBuffer() throws PersistitException { } retry++; } - throw new IllegalStateException("No available Buffers"); + return null; } enum Result { diff --git a/persistit/core/src/test/java/com/persistit/BufferPoolTest.java b/persistit/core/src/test/java/com/persistit/BufferPoolTest.java index 962cac538..6bfbb7223 100644 --- a/persistit/core/src/test/java/com/persistit/BufferPoolTest.java +++ b/persistit/core/src/test/java/com/persistit/BufferPoolTest.java @@ -1,6 +1,8 @@ /** * Copyright 2012 Akiban Technologies, Inc. - * + * + * Portions Copyrighted 2026 3A Systems, LLC. + * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at @@ -17,14 +19,22 @@ package com.persistit; import com.persistit.BufferPool.BufferHolder; +import com.persistit.exception.InUseException; +import com.persistit.exception.PersistitException; import org.junit.Test; +import java.util.ArrayList; import java.util.Arrays; +import java.util.List; import java.util.Random; import java.util.SortedSet; import java.util.TreeSet; +import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; public class BufferPoolTest extends PersistitUnitTestCase { @@ -184,4 +194,111 @@ public void testEvictVoume() throws Exception { } } + /** + * When every buffer in the pool is claimed, get() for a page that is not + * in the pool must keep retrying the allocation until the caller's timeout + * budget expires and then throw a checked InUseException — not give up + * with a raw IllegalStateException after a single clock sweep (issue + * #300). + */ + @Test + public void testGetHonorsTimeoutWhenPoolExhausted() throws Exception { + final Volume volume = _persistit.getVolume("persistit"); + final BufferPool pool = volume.getPool(); + final long timeout = 500; + final long[] pages = allocPages(volume, pool.getBufferCount() + 1); + final List claimed = new ArrayList(); + try { + InUseException exhausted = null; + long elapsed = -1; + for (final long page : pages) { + final long start = System.currentTimeMillis(); + try { + claimed.add(pool.get(volume, page, true, false, timeout)); + } catch (final InUseException e) { + exhausted = e; + elapsed = System.currentTimeMillis() - start; + break; + } + } + assertNotNull("Claiming " + claimed.size() + " buffers of " + pool.getBufferCount() + + " did not exhaust the pool", exhausted); + assertTrue("get() failed after " + elapsed + " ms, before its " + timeout + " ms timeout expired", + elapsed >= timeout - 100); + } finally { + for (final Buffer buffer : claimed) { + buffer.release(); + } + } + } + + /** + * A get() that finds the pool exhausted must keep retrying and succeed as + * soon as another thread releases a buffer, rather than failing after a + * single sweep (issue #300). + */ + @Test + public void testGetWaitsForReleasedBuffer() throws Exception { + final Volume volume = _persistit.getVolume("persistit"); + final BufferPool pool = volume.getPool(); + final long releaseDelay = 300; + final long[] pages = allocPages(volume, pool.getBufferCount() + 2); + final List claimed = new ArrayList(); + try { + /* + * Require two consecutive allocation failures so that a single + * failure caused by a transient claim held by a background thread + * is not mistaken for pool exhaustion. + */ + int consecutiveFailures = 0; + while (consecutiveFailures < 2 && claimed.size() <= pool.getBufferCount()) { + try { + claimed.add(pool.get(volume, pages[claimed.size()], true, false, 100)); + consecutiveFailures = 0; + } catch (final InUseException expected) { + consecutiveFailures++; + } + } + assertTrue("Claiming " + claimed.size() + " buffers of " + pool.getBufferCount() + + " did not exhaust the pool", claimed.size() <= pool.getBufferCount()); + + final long page = pages[pages.length - 1]; + final AtomicLong waited = new AtomicLong(-1); + final AtomicReference failure = new AtomicReference(); + final Thread getter = new Thread(new Runnable() { + @Override + public void run() { + final long start = System.currentTimeMillis(); + try { + final Buffer buffer = pool.get(volume, page, true, false, 10000); + waited.set(System.currentTimeMillis() - start); + buffer.release(); + } catch (final Throwable t) { + failure.set(t); + } + } + }, "BufferPoolTest_getter"); + getter.start(); + Thread.sleep(releaseDelay); + claimed.remove(claimed.size() - 1).release(); + getter.join(30000); + assertTrue("Getter thread did not finish", !getter.isAlive()); + assertNull("Getter thread failed: " + failure.get(), failure.get()); + assertTrue("get() returned after " + waited.get() + " ms, before a buffer was released", + waited.get() >= releaseDelay - 100); + } finally { + for (final Buffer buffer : claimed) { + buffer.release(); + } + } + } + + private long[] allocPages(final Volume volume, final int count) throws PersistitException { + final long[] pages = new long[count]; + for (int i = 0; i < count; i++) { + pages[i] = volume.getStorage().allocNewPage(); + } + return pages; + } + }