From 5d6e9a45fca57467092d6e049c5160dea20514f1 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Wed, 3 Jun 2026 18:19:35 -0400 Subject: [PATCH v16 14/21] Add client backend write combining test Test that a client backend evicting dirty buffers through a strategy ring combines contiguous dirty blocks into larger writes, both via normal victim buffer eviction and via WriteBufferAndNeighbors() directly, and that non-resident gaps fall back to single-block writes. The normal buffer eviction path is likely both too expensive and too prone to flakiness to be committable in the end, but it is nice to exercise this at least during development. --- src/backend/storage/buffer/bufmgr.c | 2 +- src/include/storage/buf_internals.h | 3 + .../modules/test_aio/t/005_write_combining.pl | 193 ++++++++++++++++++ src/test/modules/test_aio/test_aio--1.0.sql | 4 + src/test/modules/test_aio/test_aio.c | 25 +++ 5 files changed, 226 insertions(+), 1 deletion(-) diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 393971ac9b8..cb5b60c5129 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -2597,7 +2597,7 @@ InvalidateVictimBuffer(BufferDesc *buf_hdr) * with it in a single write. The victim buffer must be already pinned and * locked and will remain pinned upon return. */ -static void +void WriteBufferAndNeighbors(Buffer bufnum, BufferDesc *buf_hdr, IOContext io_context, WritebackContext *wb_context) { diff --git a/src/include/storage/buf_internals.h b/src/include/storage/buf_internals.h index 541308a678c..24c19a71220 100644 --- a/src/include/storage/buf_internals.h +++ b/src/include/storage/buf_internals.h @@ -598,6 +598,9 @@ extern StartBufferIOResult StartSharedBufferIO(BufferDesc *buf, bool forInput, b extern void TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint64 set_flag_bits, bool forget_owner, bool release_aio); +extern void WriteBufferAndNeighbors(Buffer bufnum, BufferDesc *buf_hdr, IOContext io_context, + WritebackContext *wb_context); + /* freelist.c */ extern IOContext IOContextForStrategy(BufferAccessStrategy strategy); diff --git a/src/test/modules/test_aio/t/005_write_combining.pl b/src/test/modules/test_aio/t/005_write_combining.pl index f4679a7adfc..ed41651e87c 100644 --- a/src/test/modules/test_aio/t/005_write_combining.pl +++ b/src/test/modules/test_aio/t/005_write_combining.pl @@ -27,6 +27,8 @@ my $block_size = $node->safe_psql('postgres', "SELECT current_setting('block_size')::int"); test_checkpointer_combines_writes($node, $block_size); +test_regular_backend_combines_writes($node, $block_size); +test_eager_clean_combines_writes($node, $block_size); $node->stop(); @@ -50,6 +52,20 @@ sub io_stat_writes return split /\|/, $result; } +sub io_stat_evictions +{ + my ($node, $backend_type, $context) = @_; + + return $node->safe_psql( + 'postgres', qq( + SELECT COALESCE(sum(evictions), 0)::bigint + FROM pg_stat_io + WHERE backend_type = '$backend_type' + AND object = 'relation' + AND context = '$context'; + )); +} + sub assert_combined_writes { local $Test::Builder::Level = $Test::Builder::Level + 1; @@ -64,6 +80,20 @@ sub assert_combined_writes } +sub assert_writes +{ + local $Test::Builder::Level = $Test::Builder::Level + 1; + + my ($node, $label, $backend_type, $context, $expected_writes, $expected_bytes) = @_; + my ($writes, $write_bytes, $avg_write_bytes) = + io_stat_writes($node, $backend_type, $context); + + note "$label: writes=$writes write_bytes=$write_bytes avg_write_bytes=$avg_write_bytes"; + + is($writes, $expected_writes, "$label write count"); + is($write_bytes, $expected_bytes, "$label write bytes"); +} + sub assert_writes_at_least { local $Test::Builder::Level = $Test::Builder::Level + 1; @@ -79,6 +109,18 @@ sub assert_writes_at_least "$label wrote at least $expected_bytes bytes"); } +sub assert_evictions_at_least +{ + local $Test::Builder::Level = $Test::Builder::Level + 1; + + my ($node, $label, $expected_evictions) = @_; + my $evictions = io_stat_evictions($node, 'client backend', 'normal'); + + note "$label: evictions=$evictions"; + ok($evictions >= $expected_evictions, + "$label evicted at least $expected_evictions buffers"); +} + sub assert_blocks_dirty { local $Test::Builder::Level = $Test::Builder::Level + 1; @@ -117,6 +159,32 @@ sub dirty_blocks "SELECT make_blocks_unused_dirty_flushed('$table', ARRAY[$blocks])"); } +sub allocate_until_blocks_clean +{ + my ($node, $psql, $table, $blocks, $filler, $label) = @_; + my $shared_buffers_blocks = $node->safe_psql( + 'postgres', + "SELECT pg_size_bytes(current_setting('shared_buffers')) / current_setting('block_size')::int" + ); + my $extend_by = 1024; + my $allocated = 0; + my $max_allocations = 3 * $shared_buffers_blocks; + + while ($node->safe_psql('postgres', + "SELECT true = ANY (rel_blocks_are_dirty('$table', ARRAY[$blocks]))") eq 't') + { + if ($allocated >= $max_allocations) + { + die "$label: blocks $blocks still dirty after $allocated buffer allocations"; + } + + $psql->query_safe("SELECT grow_rel('$filler'::regclass, $extend_by)"); + $allocated += $extend_by; + } + + note "$label: allocated $allocated buffers to reach regular backend victims"; + $psql->query_safe('SELECT pg_stat_force_next_flush()'); +} sub test_checkpointer_combines_writes { @@ -167,3 +235,128 @@ sub test_checkpointer_combines_writes $psql->quit(); } + +# A higher level test that might be too expensive to commit +sub test_regular_backend_combines_writes +{ + my ($node, $block_size) = @_; + my $psql = $node->background_psql('postgres', on_error_stop => 0); + + # Keep unrelated dirty buffers out of the client-backend write statistics. + $node->safe_psql('postgres', 'CHECKPOINT'); + + $node->safe_psql( + 'postgres', qq( + CREATE TABLE wc_backend (id int, payload text); + INSERT INTO wc_backend SELECT g, repeat('y', 200) FROM generate_series(1, 1000) AS g; + SELECT flush_rel_buffers('wc_backend'::regclass); + CREATE UNLOGGED TABLE wc_backend_filler (id int); + CHECKPOINT; + )); + + #### + # Test one big combined write from regular backend buffer allocation. + #### + + dirty_blocks($psql, 'wc_backend', '0,1,2,3,4,5'); + assert_blocks_dirty($node, 'wc_backend', '0,1,2,3,4,5', 't', + 'contiguous buffers are dirty before regular backend allocation'); + + flush_and_reset_io_stats($node, $psql); + allocate_until_blocks_clean($node, $psql, 'wc_backend', '0,1,2,3,4,5', + 'wc_backend_filler', 'contiguous regular backend'); + + # Assert that combining happened at all, which shouldn't fail even if the + # more specific test below it flakes. + assert_combined_writes($node, 'contiguous regular backend', 'client backend', + 'normal', $block_size); + # Unlike the checkpointer/bgwriter/eager-clean cases, we force writes here + # by growing a filler relation until the clock sweep evicts our blocks. The + # same sweep may also evict unrelated dirty pages in the 'normal' context + # (e.g. a catalog page freshly dirtied by a hint bit), so this exact write + # count test may fail. That's okay since we think this whole test routine is + # probably too expensive to commit. + assert_writes($node, 'contiguous regular backend', 'client backend', + 'normal', 1, 6 * $block_size); + assert_evictions_at_least($node, 'contiguous regular backend', 6); + assert_any_blocks_dirty($node, 'wc_backend', '0,1,2,3,4,5', 'f', + 'regular backend wrote contiguous dirty buffers'); + + #### + # Test multiple single block writes when interspersed blocks are not in + # shared buffers. + #### + + $psql->query_safe( + "SELECT invalidate_rel_blocks('wc_backend', ARRAY[1,3,5])"); + dirty_blocks($psql, 'wc_backend', '0,2,4'); + flush_and_reset_io_stats($node, $psql); + + allocate_until_blocks_clean($node, $psql, 'wc_backend', '0,2,4', + 'wc_backend_filler', 'nonresident gaps regular backend'); + # The three dirty blocks are separated by non-resident gaps, so they must + # not be combined: at least three separate writes. As above, stray dirty + # pages evicted by the same sweep may push the count higher, so assert "at + # least" rather than an exact count. + assert_writes_at_least($node, 'nonresident gaps regular backend', 'client backend', + 'normal', 3, + 3 * $block_size); + assert_evictions_at_least($node, 'nonresident gaps regular backend', 3); + assert_any_blocks_dirty($node, 'wc_backend', '0,2,4', 'f', + 'regular backend wrote dirty buffers separated by nonresident gaps'); + + $psql->quit(); +} + +sub test_eager_clean_combines_writes +{ + my ($node, $block_size) = @_; + my $psql = $node->background_psql('postgres', on_error_stop => 0); + + $node->safe_psql( + 'postgres', qq( + CREATE TABLE wc_victim (id int, payload text); + INSERT INTO wc_victim SELECT g, repeat('y', 200) FROM generate_series(1, 1000) AS g; + SELECT flush_rel_buffers('wc_victim'::regclass); + CHECKPOINT; + )); + + #### + # Test one big combined write when WriteBufferAndNeighbors() is called directly. + #### + + dirty_blocks($psql, 'wc_victim', '0,1,2,3,4,5'); + assert_blocks_dirty($node, 'wc_victim', '0,1,2,3,4,5', 't', + 'contiguous buffers are dirty before direct eager clean'); + + flush_and_reset_io_stats($node, $psql); + $psql->query_safe("SELECT eager_clean_rel_block('wc_victim', 0)"); + $psql->query_safe('SELECT pg_stat_force_next_flush()'); + + assert_writes($node, 'contiguous direct eager clean', 'client backend', + 'normal', 1, 6 * $block_size); + assert_any_blocks_dirty($node, 'wc_victim', '0,1,2,3,4,5', 'f', + 'direct eager clean wrote contiguous dirty buffers'); + + #### + # Test multiple single block writes when interspersed blocks are not in + # shared buffers. + #### + + $psql->query_safe( + "SELECT invalidate_rel_blocks('wc_victim', ARRAY[1,3,5])"); + dirty_blocks($psql, 'wc_victim', '0,2,4'); + flush_and_reset_io_stats($node, $psql); + + $psql->query_safe("SELECT eager_clean_rel_block('wc_victim', 0)"); + $psql->query_safe("SELECT eager_clean_rel_block('wc_victim', 2)"); + $psql->query_safe("SELECT eager_clean_rel_block('wc_victim', 4)"); + $psql->query_safe('SELECT pg_stat_force_next_flush()'); + assert_writes($node, 'nonresident gaps direct eager clean', 'client backend', + 'normal', 3, + 3 * $block_size); + assert_any_blocks_dirty($node, 'wc_victim', '0,2,4', 'f', + 'direct eager clean wrote dirty buffers separated by nonresident gaps'); + + $psql->quit(); +} diff --git a/src/test/modules/test_aio/test_aio--1.0.sql b/src/test/modules/test_aio/test_aio--1.0.sql index e1dc6a5ef10..d7d5006b3d5 100644 --- a/src/test/modules/test_aio/test_aio--1.0.sql +++ b/src/test/modules/test_aio/test_aio--1.0.sql @@ -45,6 +45,10 @@ CREATE FUNCTION make_blocks_unused_dirty_flushed(rel regclass, blocks int4[]) RETURNS pg_catalog.void STRICT AS 'MODULE_PATHNAME' LANGUAGE C; +CREATE FUNCTION eager_clean_rel_block(rel regclass, blockno int4) +RETURNS pg_catalog.void STRICT +AS 'MODULE_PATHNAME' LANGUAGE C; + CREATE FUNCTION rel_blocks_are_dirty(rel regclass, blocks int4[]) RETURNS pg_catalog.bool[] STRICT AS 'MODULE_PATHNAME' LANGUAGE C; diff --git a/src/test/modules/test_aio/test_aio.c b/src/test/modules/test_aio/test_aio.c index 1c6bad8e8ef..579fa3df8e4 100644 --- a/src/test/modules/test_aio/test_aio.c +++ b/src/test/modules/test_aio/test_aio.c @@ -709,6 +709,31 @@ make_blocks_unused_dirty_flushed(PG_FUNCTION_ARGS) PG_RETURN_VOID(); } +PG_FUNCTION_INFO_V1(eager_clean_rel_block); +Datum +eager_clean_rel_block(PG_FUNCTION_ARGS) +{ + Oid relid = PG_GETARG_OID(0); + BlockNumber blkno = PG_GETARG_UINT32(1); + Relation rel; + Buffer buf; + + rel = relation_open(relid, AccessShareLock); + if (RelationUsesLocalBuffers(rel)) + ereport(ERROR, + (errmsg("cannot eager clean local buffers"))); + + buf = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL, NULL); + LockBuffer(buf, BUFFER_LOCK_SHARE_EXCLUSIVE); + WriteBufferAndNeighbors(buf, GetBufferDescriptor(buf - 1), IOCONTEXT_NORMAL, + &BackendWritebackContext); + ReleaseBuffer(buf); + + relation_close(rel, NoLock); + + PG_RETURN_VOID(); +} + PG_FUNCTION_INFO_V1(rel_blocks_are_dirty); Datum rel_blocks_are_dirty(PG_FUNCTION_ARGS) -- 2.47.3