test_bufmgr - eviction algorithm buffer manager core tests#47
Open
cooltodinesh wants to merge 7 commits into
Open
test_bufmgr - eviction algorithm buffer manager core tests#47cooltodinesh wants to merge 7 commits into
cooltodinesh wants to merge 7 commits into
Conversation
Keeps gburd/postgres rebased hourly on postgres/postgres master with only .github/ changes on top (sync-upstream automatic + manual). Drops the bespoke Windows dependency-builder workflow: Windows is already built and tested in CI by upstream's pg-ci.yml (Visual Studio + MinGW meson jobs), so a separate dependency prebuild that nothing consumed was redundant.
The Open Code Review system: ocr-review and ocr-model-check workflows plus .github/ocr config (LiteLLM->Bedrock Claude Opus 4.8, rule.json, context.md, pg-history.py).
This isn't a commit that will be submitting for review, it is purely for local developer tooling while developing this patch set. Ignore it.
StrategyGetBuffer() advances the shared clock hand, nextVictimBuffer, with a pg_atomic_fetch_add_u32(..., 1) on every tick. On a multi-socket system the cache line holding that counter has to travel over the interconnect on each operation, pushing a sweep tick from ~20ns (same-socket, line warm in L1/L2) into the ~100-200ns range. Under eviction pressure with hundreds of backends in StrategyGetBuffer() concurrently, that single cache line becomes the dominant cost of the sweep, visible as elevated bus-cycles and cache-misses in a perf profile. Have each backend claim a run of consecutive buffer IDs from the shared hand with a single fetch-add and then iterate through them privately. The sweep still advances through the pool in order, each buffer is still visited exactly once per complete pass, and the meaning of the clock state is unchanged; only the temporal ordering of visits within a pass changes, which the algorithm does not depend on. The contended atomic now fires roughly once per batch rather than once per buffer. The batch is one cache line's worth of clock-hand values -- PG_CACHE_LINE_SIZE / sizeof(uint32) -- capped at NBuffers so a claim can never wrap the pool more than once. Batching only helps when the counter's cache line actually bounces between sockets, so it is enabled only on multi-node NUMA hardware (pg_numa_get_max_node() >= 1); on a single socket, or where libnuma is unavailable, the batch size stays 1 and the code path is byte-identical to the stock clock sweep. Wraparound handling is adjusted: with batching, several backends can each see a fetch-add return a value past NBuffers within the same pass. Any such backend takes buffer_strategy_lock, re-reads the counter, and if it is still out of range wraps it with a single CAS and increments completePasses. StrategySyncStart() continues to see a consistent (nextVictimBuffer, completePasses) pair. This is the batched-clock-sweep idea from Jim Mlodgenski's pgsql-hackers thread, adapted to derive the batch size from the platform cache-line size. Co-authored-by: Jim Mlodgenski <mlodj@amazon.com>
Replace the 0..5 usage_count buffer-replacement policy with a cooling-stage
clock (the LeanStore / 2Q-A1 model): a buffer is simply HOT (recently used)
or COOL (an eviction candidate), with "pinned" being the existing refcount.
There is no per-buffer access counter.
- A demand-loaded page is admitted COOL (probationary), not HOT. A second
access via PinBuffer promotes it COOL -> HOT (the rescue). So a page
touched once -- a sequential scan -- fills and drains the COOL stage and
is evicted from it without ever displacing the HOT working set. Scan
resistance is intrinsic to the replacement algorithm, which is what lets a
later commit remove the BufferAccessStrategy ring buffers entirely.
- The foreground sweep in StrategyGetBuffer() reclaims an already-COOL,
unpinned buffer, pinning it with a CAS so a racing PinBuffer always wins.
Promotion (COOL -> HOT) and demotion (HOT -> COOL) are single-bit
transitions; only the eviction claim is a CAS.
- The background writer maintains the supply of COOL victims. As its LRU
scan runs ahead of the clock hand it demotes HOT buffers to COOL, so the
foreground finds a victim in a single pass rather than having to cool
buffers itself. The demotion is demand-driven -- bounded by the predicted
allocation for the next cycle -- so it stages just enough COOL buffers
without cooling the whole pool, and it is done under the buffer header
lock the scan already holds. A single second-chance reference bit
(set by PinBuffer, cleared on the bgwriter's first pass over a HOT buffer)
spares a recently-accessed buffer one cooling pass, keeping the genuinely
hot set out of the COOL stage under scan pressure. BgBufferSync also
tracks the reusable-buffer density it directly observes on a shorter
smoothing window, so a burst of probationary/scan COOL pages is followed
promptly rather than averaged away. Under a bulk-dirtying workload the
per-cycle clean-write cap is raised from bgwriter_lru_maxpages to predicted
demand so the bgwriter keeps supplying clean victims, rather than the
foreground sweep having to flush dirty victims inline; normal workloads,
where demand is below the cap, are unaffected.
The 4-bit usage_count field is reinterpreted in place: bit 0 is the HOT/COOL
state (BUF_COOLSTATE_ONE), bit 1 the reference bit (BUF_REFBIT). The 64-bit
buffer-state layout -- refcount, flag and lock offsets and their StaticAsserts
-- is unchanged; only the meaning of the field and the instructions that touch
it change. A StaticAssert requires the field to be at least 2 bits wide so a
future width change cannot push the reference bit into the flag bits.
BM_MAX_USAGE_COUNT becomes BUF_COOLSTATE_HOT (1), so the pin fast path
saturates at HOT. Local (temp-table) buffers get the same two-state
treatment; being single-backend they need no background cooler.
Because the reference bit shares the field, the full 4-bit value can be 0..3;
readers that mean "the cooling state" must use BUF_STATE_GET_COOLSTATE(), which
masks to bit 0 and returns only 0 (COOL) or 1 (HOT), never the raw field.
contrib/pg_buffercache is updated accordingly: its usagecount column and the
pg_buffercache_summary average report the cooling state (0 = COOL, 1 = HOT),
and pg_buffercache_usage_counts() buckets on it. Using the raw 4-bit getter
there would index its BM_MAX_USAGE_COUNT+1 = 2-element arrays with values up
to 3 and overrun the stack; masking to the cooling bit keeps the index in
range. The reference bit is deliberately not exposed as usagecount.
Depends on the batched clock sweep from the previous commit.
The cooling-stage evictor admits demand-loaded pages COOL and promotes them to
HOT only on a second access, so a one-touch sequential scan fills and drains
the COOL stage without displacing the hot working set. Scan resistance is
therefore a property of the replacement algorithm itself, and the
BufferAccessStrategy ring buffers that previously provided it are dead weight.
Remove them end to end.
Deleted:
- the BufferAccessStrategy type and the BufferAccessStrategyType enum
(BAS_NORMAL/BULKREAD/BULKWRITE/VACUUM);
- the ring machinery in freelist.c (GetAccessStrategy[WithSize],
GetAccessStrategyBufferCount, GetAccessStrategyPinLimit,
FreeAccessStrategy, GetBufferFromRing, AddBufferToRing,
StrategyRejectBuffer, IOContextForStrategy);
- the strategy parameter from ReadBufferExtended, ReadBufferWithoutRelcache,
the ExtendBufferedRel* family, StrategyGetBuffer, read_stream_begin_*,
and every scan/vacuum/analyze/index-AM caller;
- the strategy fields on HeapScanDescData, IndexScanDescData,
BulkInsertStateData, ReadBuffersOperation, and ReadStream;
- _hash_getbuf_with_strategy (identical to _hash_getbuf without a strategy).
pg_stat_io's per-strategy IO contexts collapse: IOCONTEXT_BULKREAD,
IOCONTEXT_BULKWRITE and IOCONTEXT_VACUUM are removed, leaving IOCONTEXT_INIT
and IOCONTEXT_NORMAL. IOOP_REUSE only ever occurred while recycling a ring
buffer, so it is no longer tracked; GetVictimBuffer counts IOOP_EVICT only.
The vacuum_buffer_usage_limit GUC and the VACUUM/ANALYZE (BUFFER_USAGE_LIMIT
...) option are removed, along with the VacuumBufferUsageLimit global, the
ring-size plumbing through VacuumParams and parallel vacuum, and vacuumdb's
--buffer-usage-limit client option. read_stream's per-backend pin budget is
now enforced solely by GetPinLimit()/GetLocalPinLimit(), which already applied
and is unchanged for the (formerly universal) no-strategy case.
Documentation and the stats/amcheck regression tests are updated to drop the
removed contexts and options.
Per Greg Burd's review on PR gburd#44 (gburd/postgres), grow the existing bcs-only test_bufmgr module into a durable, algorithm-agnostic test suite for the buffer manager subsystem. This commit is the Tier-1 half: properties any correct buffer manager must have, expressed only through observable behavior (residency, contents, pins), never through internal field encodings. The Tier-2 half -- mechanics specific to this branch's HOT/COOL cooling-stage evictor -- has been split onto feat/test-bufmgr-cooling, so a future replacement algorithm touches only that branch and this one stays untouched. Seven regress tests and one isolation spec: - eviction_reload: an evicted page reloads with correct contents, and the manager makes progress under pressure. - dirty_persistence: an evicted dirty page reloads with its modified contents (no checkpoint or restart needed -- eviction itself forces the write-out). - scan_no_retain: a one-touch page does not indefinitely retain its buffer under pressure. - truncate_stale / drop_recreate_stale: TRUNCATE and DROP+recreate never let stale cached pages resurface. - local_eviction: the local (temp-table) buffer pool reuses correctly, sized from temp_buffers in bytes. - churn: contents stay correct after many replacement cycles over a working set several times the pool. - pinned_safety (isolation): a buffer pinned via a suspended cursor in one session survives a concurrent flood in another, with an unpinned control probe confirming the flood actually exceeded the pool. Two things make this portable rather than bcs-specific: - Pressure is generated with pg_prewarm in 'buffer' mode, which pulls blocks into shared_buffers through the normal replacement path with no BufferAccessStrategy ring -- unlike a plain sequential scan, which would use the ring on stock master and so never cycle the whole pool there. - Fixed-size working sets are derived from the relevant GUC in bytes (shared_buffers / temp_buffers times block_size) rather than a hand-tuned row count, so they hold at any BLCKSZ. A scan-resistance test ("a hot page survives a one-touch flood") was attempted and dropped: it is not a universal property (fails on stock master, where the bulk-read ring rather than an aging policy provides scan resistance) and is geometry-dependent and non-monotonic even on this branch. It's recorded as a TODO in the README pending injection-point control over buffer placement. Verified green and deterministic (4+ repeated runs, no flakiness) on: this branch (bcs), stock master (20devel), REL_17_STABLE, and at BLCKSZ 4/8/16/32 kB. NOTICE output from CREATE EXTENSION IF NOT EXISTS and DROP ... IF EXISTS is suppressed via client_min_messages=warning in the test config, so expected output isn't coupled to schedule order. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
cooltodinesh
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to your review on #44. This grows the
test_bufmgrmodule intotier-1/core properties any correct buffer manager must have, expressed only
through observable behavior (residency, contents, pins), never through
internal field encodings. Tier 2 (bcs-specific HOT/COOL mechanics from #44,
reading the coolstate bit) has been split onto a separate branch/PR so it
can ride with this series while Tier 1 stands on its own.
What's here
Seven regress tests and one isolation spec:
the manager makes progress under pressure (no "no unpinned buffers" error).
contents. No checkpoint or restart needed: eviction itself forces the
write-out (verified directly that
CHECKPOINTclears the dirty bitwithout evicting, so a background checkpoint stealing the write would be
silent, not a failure — see README for the exact mechanism).
buffer under pressure (the universal half of scan resistance).
never let stale cached pages resurface.
sized from
temp_buffersin bytes so it holds at any BLCKSZ.working set several times the pool.
one session survives a concurrent flood in another session, with an
unpinned control probe confirming the flood actually exceeded the pool
(guards against a vacuous pass).
What makes this portable rather than bcs-specific
pg_prewarm('rel', 'buffer'), not plain scans. Prewarm'sbuffer mode pulls blocks in through the normal replacement path with no
BufferAccessStrategyring, unlike a sequential scan/COPY, which woulduse the ring on stock master and so never cycle the whole pool there.
shared_buffers/temp_buffers×block_size), not a hand-tuned row count, so working sets exceed thepool at any BLCKSZ rather than at one config.
the observed end-state occurs (a tracked block goes non-resident),
bounded by a generous round cap, instead of asserting a fixed size
happened to be right.
Scan resistance: attempted and dropped
I tried a Tier-1 "hot page survives a one-touch flood" test and pulled it
after validating on stock master and across BLCKSZ:
against a greater-than-pool flood at the test pool size (the flood's own
bulk read/write pressure churns the pool there); it's a property of a
scan-resistant policy like this branch's cooling evictor, not of every
correct buffer manager.
same 8MB pool the hot page survived at 1024 buffers (8kB BLCKSZ) but was
evicted at both 256 (32kB) and 2048 (4kB) buffers, because the
one-reprieve evictor drops a page whose slot a single greater-than-pool
sweep revisits a second time, regardless of how hot it is.
It's recorded as a TODO in the README — a real version needs
injection-point control over buffer placement (or a comparative hit-ratio
measure), not a single post-flood residency check.
Verified
(bcs), stock master (currently
20devel), andREL_17_STABLE.NOTICEoutput fromCREATE EXTENSION IF NOT EXISTS/DROP ... IF EXISTSis suppressed viaclient_min_messages = warningin the testconfig (server-level, not a per-statement
SET), so expected outputisn't coupled to schedule order.
Happy to iterate based on your read of the scan-resistance tradeoff, or on
anything else.