Skip to content

[#300] Make BufferPool.get honor its timeout when allocating a buffer #301

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vharseko merged 1 commit into from
Jul 23, 2026
Merged

[#300] Make BufferPool.get honor its timeout when allocating a buffer #301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 42 additions & 25 deletions persistit/core/src/main/java/com/persistit/BufferPool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (;;) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -1070,7 +1087,7 @@ private Buffer allocBuffer() throws PersistitException {
}
retry++;
}
throw new IllegalStateException("No available Buffers");
return null;
}

enum Result {
Expand Down
119 changes: 118 additions & 1 deletion persistit/core/src/test/java/com/persistit/BufferPoolTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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<Buffer> claimed = new ArrayList<Buffer>();
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<Buffer> claimed = new ArrayList<Buffer>();
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<Throwable> failure = new AtomicReference<Throwable>();
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;
}

}
Loading