Skip to content

persistit: BufferPool.allocBuffer gives up after one clock sweep, ignoring the get() timeout budget #300

Description

@vharseko

Symptom

TransactionTest2.transactionsWithInterrupts failed once on a master push build, cell build-maven (windows-latest, 26) (the other 8 cells were green): run 30011500179.

One worker thread (of ~200 spawned across the 10-second interrupt storm) died on a plain read:

java.lang.IllegalStateException: No available Buffers
	at com.persistit.BufferPool.allocBuffer(BufferPool.java:1073)
	at com.persistit.BufferPool.get(BufferPool.java:784)
	at com.persistit.Exchange.searchLevel(Exchange.java:1289)
	at com.persistit.Exchange.searchTree(Exchange.java:1194)
	at com.persistit.Exchange.search(Exchange.java:1120)
	at com.persistit.Exchange.searchAndFetchInternal(Exchange.java:3178)
	at com.persistit.Exchange.fetch(Exchange.java:3091)
	at com.persistit.Exchange.fetch(Exchange.java:2948)
	at com.persistit.TransactionTest2.transfer(TransactionTest2.java:371)
	at com.persistit.TransactionTest2.runIt(TransactionTest2.java:313)

The account balance still agreed (Ending balance is 0 which AGREES) and the ATC assertion passed; the test failed only on the worker-death assertion added by #288:

TransactionTest2.transactionsWithInterrupts:246 Worker threads died with unexpected
exceptions; first was java.lang.IllegalStateException: No available Buffers
expected:<0> but was:<1>

Before #288 this worker death would have been swallowed silently (stack trace in the log, green test), so this is a pre-existing condition made visible on purpose — not a new regression. In particular it is not caused by #295, which merged just before this run: its BufferPool.java change is a comment in the constructor's OOME diagnostic path, and its Exchange change (fetchBufferCopy) is not on the failing path.

Root cause

BufferPool.get(Volume, long, boolean, boolean, long timeout) has a timeout budget (60 s default via SharedResource.DEFAULT_MAX_WAIT_TIME; Exchange.searchLevel passes _timeoutMillis), but the budget is honored only on the mustClaim path (claiming a buffer that already holds the wanted page). The allocation path never consults it: on a pool miss, allocBuffer() performs a single clock sweep of at most 2 × _bufferCount ticks and then throws a raw IllegalStateException("No available Buffers").

A sweep tick is wasted whenever the inspected buffer is:

  • touched — the tick only clears the touched bit, and hot buffers get re-touched immediately;
  • claimed by anyone (CLAIMED_MASK != 0) — reader claims from concurrent tree walks count;
  • not acquirable via the non-blocking claim(true, 0);
  • valid but not detachable at that moment.

The clock (_clock) is shared, so concurrent sweepers advance it past each other's candidates and race for the same evictable buffer — an individual thread's sweep does not even examine every buffer once.

The unit-test configuration makes the window reachable: the pool has only 20 buffers (buffer.count.16384=20 in PersistitUnitTestCase), i.e. a 40-tick give-up threshold, while 8 worker threads hammer a small hot tree and the interrupt storm generates extra churn (aborted MVVs → CleanupManager pruning, dirty pages, replacement threads re-faulting their working set; the failing run logged Retried transactions: 5705). On a slow, loaded Windows CI VM a thread can stochastically burn all 40 ticks and throw.

This is inherited Akiban-era behavior. It is not test-only: any production deployment with a small/overloaded buffer pool can get a spurious IllegalStateException from a read instead of waiting for a buffer within the caller's timeout budget.

Proposed fix

Make the allocation path honor the timeout parameter of BufferPool.get:

  • On sweep exhaustion, retry the whole get() loop with a short backoff until timeout expires, instead of failing after a single sweep. The retry belongs at the get() level (its finally has already released the hash lock there), not inside allocBuffer(), so no thread sleeps while holding a hash-lock stripe.
  • If the budget is truly exhausted, throw a meaningful checked persistit exception (e.g. an InUseException-style timeout) rather than a raw IllegalStateException.

Environment

Related: #286, #288, #268.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions