public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v9 2/3] test 6+ messages / 4 participants [nested] [flat]
* [PATCH v9 2/3] test @ 2022-01-23 17:47 Michail Nikolaev <[email protected]> 0 siblings, 0 replies; 6+ messages in thread From: Michail Nikolaev @ 2022-01-23 17:47 UTC (permalink / raw) --- src/test/recovery/Makefile | 1 + .../recovery/t/027_standby_index_lp_dead.pl | 372 ++++++++++++++++++ 2 files changed, 373 insertions(+) create mode 100644 src/test/recovery/t/027_standby_index_lp_dead.pl diff --git a/src/test/recovery/Makefile b/src/test/recovery/Makefile index 4eb12e0583..84a5eb5453 100644 --- a/src/test/recovery/Makefile +++ b/src/test/recovery/Makefile @@ -10,6 +10,7 @@ #------------------------------------------------------------------------- EXTRA_INSTALL=contrib/test_decoding +EXTRA_INSTALL+=contrib/pageinspect subdir = src/test/recovery top_builddir = ../../.. diff --git a/src/test/recovery/t/027_standby_index_lp_dead.pl b/src/test/recovery/t/027_standby_index_lp_dead.pl new file mode 100644 index 0000000000..5237d7603c --- /dev/null +++ b/src/test/recovery/t/027_standby_index_lp_dead.pl @@ -0,0 +1,372 @@ +# Checks that index hints on standby work as excepted. +use strict; +use warnings; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; +use Config; + +plan tests => 30; + +# Initialize primary node +my $node_primary = PostgreSQL::Test::Cluster->new('primary'); +$node_primary->init(allows_streaming => 1); +$node_primary->append_conf('postgresql.conf', qq{ + autovacuum = off + enable_seqscan = off + enable_indexonlyscan = off + checkpoint_timeout = 1h +}); +$node_primary->start; + +$node_primary->safe_psql('postgres', 'CREATE EXTENSION pageinspect'); +# Create test table with primary index +$node_primary->safe_psql( + 'postgres', 'CREATE TABLE test_table (id int, value int)'); +$node_primary->safe_psql( + 'postgres', 'CREATE INDEX test_index ON test_table (value, id)'); +# Fill some data to it, note to not put a lot of records to avoid +# heap_page_prune_opt call which cause conflict on recovery hiding conflict +# caused due index hint bits +$node_primary->safe_psql('postgres', + 'INSERT INTO test_table VALUES (generate_series(1, 30), 0)'); +# And vacuum to allow index hint bits to be set +$node_primary->safe_psql('postgres', 'VACUUM test_table'); +# For fail-fast in case FPW from primary +$node_primary->safe_psql('postgres', 'CHECKPOINT'); + +# Take backup +my $backup_name = 'my_backup'; +$node_primary->backup($backup_name); + +# Restore standby node from backup backup +my $node_standby_1 = PostgreSQL::Test::Cluster->new('standby_1'); +$node_standby_1->init_from_backup($node_primary, $backup_name, + has_streaming => 1); + +my $standby_settings = qq{ + max_standby_streaming_delay = 1 + wal_receiver_status_interval = 1 + hot_standby_feedback = off + autovacuum = off + enable_seqscan = off + enable_indexonlyscan = off + checkpoint_timeout = 1h +}; +$node_standby_1->append_conf('postgresql.conf', $standby_settings); +$node_standby_1->start; + +$node_standby_1->backup($backup_name); + +# Create second standby node linking to standby 1 +my $node_standby_2 = PostgreSQL::Test::Cluster->new('standby_2'); +$node_standby_2->init_from_backup($node_standby_1, $backup_name, + has_streaming => 1); +$node_standby_2->append_conf('postgresql.conf', $standby_settings); +$node_standby_2->start; + +# To avoid hanging while expecting some specific input from a psql +# instance being driven by us, add a timeout high enough that it +# should never trigger even on very slow machines, unless something +# is really wrong. +my $psql_timeout = IPC::Run::timer(300); + +# One psql to run command in repeatable read isolation level. +# It is used to test xactStartedInRecovery snapshot after promotion. +# Also, it is used to check fact what active snapshot on standby prevent LP_DEAD +# to be set (ComputeXidHorizons work on standby). +my %psql_standby_repeatable_read = ('stdin' => '', 'stdout' => '', 'stderr' => ''); +$psql_standby_repeatable_read{run} = + IPC::Run::start( + [ 'psql', '-XAb', '-f', '-', '-d', $node_standby_1->connstr('postgres') ], + '<', \$psql_standby_repeatable_read{stdin}, + '>', \$psql_standby_repeatable_read{stdout}, + '2>', \$psql_standby_repeatable_read{stderr}, + $psql_timeout); + +# Another psql to run command in read committed isolation level +my %psql_standby_read_committed = ('stdin' => '', 'stdout' => '', 'stderr' => ''); +$psql_standby_read_committed{run} = + IPC::Run::start( + [ 'psql', '-XAb', '-f', '-', '-d', $node_standby_1->connstr('postgres') ], + '<', \$psql_standby_read_committed{stdin}, + '>', \$psql_standby_read_committed{stdout}, + '2>', \$psql_standby_read_committed{stderr}, + $psql_timeout); + +# Start RR transaction and read first row from index +ok(send_query_and_wait(\%psql_standby_repeatable_read, + q[ +BEGIN TRANSACTION ISOLATION LEVEL REPEATABLE READ; +SELECT id FROM test_table WHERE value = 0 ORDER BY id LIMIT 1; +], + qr/1\n\(1 row\)/m), + 'row is visible in repeatable read'); + +# Start RC transaction and read first row from index +ok(send_query_and_wait(\%psql_standby_read_committed, + q[ +BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED; +SELECT id FROM test_table WHERE value = 0 ORDER BY id LIMIT 1; +], + qr/1\n\(1 row\)/m), + 'row is visible in read committed'); + +# Now delete first 10 rows in index +$node_primary->safe_psql('postgres', + 'UPDATE test_table SET value = 1 WHERE id <= 10'); + +# Make sure hint bits are not set on primary yet +is(hints_num($node_primary), qq(0), 'no index hint bits are set on primary yet'); + +# Make sure page is not processed by heap_page_prune_opt +# (to avoid false positive results) +is(non_normal_num($node_primary), qq(0), 'all items are normal in heap'); + +# Wait for standbys to catch up transaction +wait_for_catchup_all(); + +is(hints_num($node_standby_1), qq(0), 'no index hint bits are set on standby 1 yet'); +is(hints_num($node_standby_2), qq(0), 'no index hint bits are set on standby 2 yet'); + +# Try to set hint bits in index on standbys +try_to_set_hint_bits($node_standby_1); +try_to_set_hint_bits($node_standby_2); + +# Make sure previous queries not set the hints on standby because +# of RR snapshot on standby 1 +is(hints_num($node_standby_1), qq(0), 'no index hint bits are set on standby 1 yet'); +is(btp_safe_on_stanby($node_standby_1), qq(0), 'hint are not marked as standby-safe'); + +# At the same time hint bits are set on second standby +is(hints_num($node_standby_2), qq(10), 'index hint bits already set on standby 2'); +is(btp_safe_on_stanby($node_standby_2), qq(1), 'hints are marked as standby-safe'); + +# Make sure read committed transaction is able to see correct data +ok(send_query_and_wait(\%psql_standby_read_committed, + q/SELECT id FROM test_table WHERE value = 0 ORDER BY id LIMIT 1;/, + qr/11\n\(1 row\)/m), + 'row is not visible in read committ'); + +# The same check for repeatable read transaction +ok(send_query_and_wait(\%psql_standby_repeatable_read, + q/SELECT id FROM test_table WHERE value = 0 ORDER BY id LIMIT 1;/, + qr/1\n\(1 row\)/m), + 'row is visible in repeatable read'); + +# Make checkpoint to cause FPI by LP_DEAD on primary +$node_primary->safe_psql('postgres', "CHECKPOINT"); + +# Set index hint bits and replicate to standby as FPI +$node_primary->safe_psql('postgres', + 'SELECT id FROM test_table WHERE value = 0 ORDER BY id LIMIT 1;'); + +# Make sure page is not processed by heap_page_prune_opt to avoid false +# positive test results +is(non_normal_num($node_primary), qq(0), 'all items are normal in heap'); +# Make sure hint bits are set +is(hints_num($node_primary), qq(10), 'hint bits are set on primary already'); + +## Wait for standbys to catch up hint bits +wait_for_catchup_all(); + +is(hints_num($node_standby_1), qq(10), 'hints are set on standby 1 because FPI'); +is(btp_safe_on_stanby($node_standby_1), qq(0), 'hints are not marked as standby-safe'); + +is(hints_num($node_standby_2), qq(10), 'hints are set on standby 2 because FPI'); +is(btp_safe_on_stanby($node_standby_2), qq(0), 'hints are not marked as standby-safe'); + +# Make sure read committed transaction is able to see correct data +ok(send_query_and_wait(\%psql_standby_read_committed, + q/SELECT id FROM test_table WHERE value = 0 ORDER BY id LIMIT 1;/, + qr/11\n\(1 row\)/m), + 'row is not visible in read committed'); + +# Make sure repeatable read transaction able to see correct data +# because hint bits are marked as non-safe +ok(send_query_and_wait(\%psql_standby_repeatable_read, + q/SELECT id FROM test_table WHERE value = 0 ORDER BY id LIMIT 1;/, + qr/1\n\(1 row\)/m), + 'row is visible in repeatable read'); + +$node_primary->stop(); + +# promote standby to new primary +$node_standby_1->promote(); +my $node_new_primary = $node_standby_1; + +# Make sure read committed transaction is able to see correct data +ok(send_query_and_wait(\%psql_standby_read_committed, + q/SELECT id FROM test_table WHERE value = 0 ORDER BY id LIMIT 1;/, + qr/11\n\(1 row\)/m), + 'row is not visible in read committed after promote'); + +# Make sure repeatable read transaction able to see correct data +# because hint bits are marked as non-safe and transaction was started on standby +ok(send_query_and_wait(\%psql_standby_repeatable_read, + q/SELECT id FROM test_table WHERE value = 0 ORDER BY id LIMIT 1;/, + qr/1\n\(1 row\)/m), + 'row is visible in repeatable read after promote'); + +# explicitly shut down psql instances gracefully - to avoid hangs +# or worse on windows +$psql_standby_read_committed{stdin} .= "ROLLBACK;\n"; +$psql_standby_repeatable_read{stdin} .= "ROLLBACK;\n"; +$psql_standby_read_committed{stdin} .= "\\q\n"; +$psql_standby_repeatable_read{stdin} .= "\\q\n"; +$psql_standby_read_committed{run}->finish; +$psql_standby_repeatable_read{run}->finish; + +# Remove one more row +$node_new_primary->safe_psql('postgres', + 'UPDATE test_table SET value = 1 WHERE id <= 11'); + +# Set one more index hint bit as on primary +$node_new_primary->safe_psql('postgres', + 'SELECT id FROM test_table WHERE value = 0 ORDER BY id LIMIT 1;'); +is(hints_num($node_new_primary), qq(11), 'hint bits are set on new primary already'); +# Checkpoint before backup +$node_new_primary->safe_psql('postgres', "CHECKPOINT"); + +my $new_backup_name = 'my_new_backup'; +$node_new_primary->backup($new_backup_name); + +# Create third standby node linking to promoted primary +my $node_new_standby = PostgreSQL::Test::Cluster->new('standby_3'); +$node_new_standby->init_from_backup($node_new_primary, $new_backup_name, + has_streaming => 1); +$node_new_standby->append_conf('postgresql.conf', $standby_settings); +$node_new_standby->start; + +is(hints_num($node_new_standby), qq(11), 'hint bits are from backup on new standby'); +is(btp_safe_on_stanby($node_new_standby), qq(0), 'hint not marked as standby-safe'); + +# Required for stability - make sure at lest LOG_SNAPSHOT_INTERVAL_MS before +# next XLOG_RUNNING_XACTS. XLOG_RUNNING_XACTS causes minRecoveryPoint to processed +# and breaks test logic. +my $xlog_running_xacts_lsn = wait_for_xlog_running_xacts($node_new_primary); +# Wait XLOG_RUNNING_XACTS applied to standby +$node_new_primary->wait_for_catchup($node_new_standby, 'replay', $xlog_running_xacts_lsn); + +# Remove one more row and get index page LSN > minRecoveryPoint +$node_new_primary->safe_psql('postgres', + 'UPDATE test_table SET value = 1 WHERE id <= 12'); +$node_new_primary->wait_for_catchup($node_new_standby, 'replay', + $node_new_primary->lsn('insert')); + +is(btp_safe_on_stanby($node_new_standby), qq(0), 'hint from FPI'); + +# Make sure bits are set only if minRecoveryPoint > than index page LSN +try_to_set_hint_bits($node_new_standby); +is(hints_num($node_new_standby), qq(11), 'no new index hint bits are set on new standby'); +is(btp_safe_on_stanby($node_new_standby), qq(0), 'hint not marked as standby-safe'); + +# Issue checkpoint on primary to update minRecoveryPoint on standby +$node_new_primary->safe_psql('postgres', "CHECKPOINT"); +$node_new_primary->wait_for_catchup($node_new_standby, 'replay', + $node_new_primary->lsn('insert')); + +# Clear hint bits from base backup and set own (now index page LSN < minRecoveryPoint) +try_to_set_hint_bits($node_new_standby); +is(hints_num($node_new_standby), qq(12), 'hint bits are set on new standby'); +is(btp_safe_on_stanby($node_new_standby), qq(1), 'hint now marked as standby-safe'); + +$node_new_primary->stop(); +$node_standby_2->stop(); +$node_new_standby->stop(); + +# Send query, wait until string matches +sub send_query_and_wait { + my ($psql, $query, $untl) = @_; + + # send query + $$psql{stdin} .= $query; + $$psql{stdin} .= "\n"; + + # wait for query results + $$psql{run}->pump_nb(); + while (1) { + # See PostgreSQL::Test::Cluster.pm's psql() + $$psql{stdout} =~ s/\r\n/\n/g if $Config{osname} eq 'msys'; + + last if $$psql{stdout} =~ /$untl/; + + if ($psql_timeout->is_expired) { + BAIL_OUT("aborting wait: program timed out \n" . + "stream contents: >>$$psql{stdout}<< \n" . + "pattern searched for: $untl\n"); + return 0; + } + if (not $$psql{run}->pumpable()) { + BAIL_OUT("aborting wait: program died\n" + . "stream contents: >>$$psql{stdout}<<\n" + . "pattern searched for: $untl\n"); + return 0; + } + $$psql{run}->pump(); + select(undef, undef, undef, 0.01); # sleep a little + + } + + $$psql{stdout} = ''; + + return 1; +} + +sub try_to_set_hint_bits { + my ($node) = @_; + # Try to set hint bits in index on standby + foreach (0 .. 10) { + $node->safe_psql('postgres', + 'SELECT * FROM test_table WHERE value = 0 ORDER BY id LIMIT 1;'); + } +} + +sub wait_for_catchup_all { + $node_primary->wait_for_catchup($node_standby_1, 'replay', + $node_primary->lsn('insert')); + $node_standby_1->wait_for_catchup($node_standby_2, 'replay', + $node_standby_1->lsn('replay')); +} + +sub hints_num { + my ($node) = @_; + return $node->safe_psql('postgres', + "SELECT count(*) FROM bt_page_items('test_index', 1) WHERE dead = true"); +} + +sub btp_safe_on_stanby { + # BTP_LP_SAFE_ON_STANDBY (1 << 9) + my ($node) = @_; + if ($node->safe_psql('postgres', + "SELECT btpo_flags FROM bt_page_stats('test_index', 1);") & (1 << 9)) { + return 1 + } else { + return 0 + } +} + +sub non_normal_num { + my ($node) = @_; + return $node->safe_psql('postgres', + "SELECT COUNT(*) FROM heap_page_items(get_raw_page('test_table', 0)) WHERE lp_flags != 1"); +} + +sub wait_for_xlog_running_xacts { + my ($node) = @_; + my ($before); + $before = $node->safe_psql('postgres', "SELECT pg_current_wal_lsn();"); + # Max wait is LOG_SNAPSHOT_INTERVAL_MS + while (1) { + sleep(1); + my $now = $node->safe_psql('postgres', "SELECT pg_current_wal_lsn();"); + if ($now ne $before) { + return $now; + } + if ($psql_timeout->is_expired) { + BAIL_OUT("program timed out waiting for XLOG_RUNNING_XACTS\n"); + return 0; + } + } +} \ No newline at end of file -- 2.33.1 --2nywz2fa4doeh6ob Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="v9-0003-docs.patch" ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: per backend I/O statistics @ 2024-09-03 06:37 Kyotaro Horiguchi <[email protected]> 0 siblings, 1 reply; 6+ messages in thread From: Kyotaro Horiguchi @ 2024-09-03 06:37 UTC (permalink / raw) To: [email protected]; +Cc: [email protected] At Mon, 2 Sep 2024 14:55:52 +0000, Bertrand Drouvot <[email protected]> wrote in > Hi hackers, > > Please find attached a patch to implement $SUBJECT. > > While pg_stat_io provides cluster-wide I/O statistics, this patch adds a new > pg_my_stat_io view to display "my" backend I/O statistics and a new > pg_stat_get_backend_io() function to retrieve the I/O statistics for a given > backend pid. > > By having the per backend level of granularity, one could for example identify > which running backend is responsible for most of the reads, most of the extends > and so on... The pg_my_stat_io view could also be useful to check the > impact on the I/O made by some operations, queries,... in the current session. > > Some remarks: > > - it is split in 2 sub patches: 0001 introducing the necessary changes to provide > the pg_my_stat_io view and 0002 to add the pg_stat_get_backend_io() function. > - the idea of having per backend I/O statistics has already been mentioned in > [1] by Andres. > > Some implementation choices: > > - The KIND_IO stats are still "fixed amount" ones as the maximum number of > backend is fixed. > - The statistics snapshot is made for the global stats (the aggregated ones) and > for my backend stats. The snapshot is not build for all the backend stats (that > could be memory expensive depending on the number of max connections and given > the fact that PgStat_IO is 16KB long). > - The above point means that pg_stat_get_backend_io() behaves as if > stats_fetch_consistency is set to none (each execution re-fetches counters > from shared memory). > - The above 2 points are also the reasons why the pg_my_stat_io view has been > added (as its results takes care of the stats_fetch_consistency setting). I think > that makes sense to rely on it in that case, while I'm not sure that would make > a lot of sense to retrieve other's backend I/O stats and taking care of > stats_fetch_consistency. > > > [1]: https://www.postgresql.org/message-id/20230309003438.rectf7xo7pw5t5cj%40awork3.anarazel.de I'm not sure about the usefulness of having the stats only available from the current session. Since they are stored in shared memory, shouldn't we make them accessible to all backends? However, this would introduce permission considerations and could become complex. When I first looked at this patch, my initial thought was whether we should let these stats stay "fixed." The reason why the current PGSTAT_KIND_IO is fixed is that there is only one global statistics storage for the entire database. If we have stats for a flexible number of backends, it would need to be non-fixed, perhaps with the entry for INVALID_PROC_NUMBER storing the global I/O stats, I suppose. However, one concern with that approach would be the impact on performance due to the frequent creation and deletion of stats entries caused by high turnover of backends. Just to be clear, the above comments are not meant to oppose the current implementation approach. They are purely for the sake of discussing comparisons with other possible approaches. regards. -- Kyotaro Horiguchi NTT Open Source Software Center ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: per backend I/O statistics @ 2024-09-03 07:07 Kyotaro Horiguchi <[email protected]> parent: Kyotaro Horiguchi <[email protected]> 0 siblings, 1 reply; 6+ messages in thread From: Kyotaro Horiguchi @ 2024-09-03 07:07 UTC (permalink / raw) To: [email protected]; +Cc: [email protected] At Tue, 03 Sep 2024 15:37:49 +0900 (JST), Kyotaro Horiguchi <[email protected]> wrote in > When I first looked at this patch, my initial thought was whether we > should let these stats stay "fixed." The reason why the current > PGSTAT_KIND_IO is fixed is that there is only one global statistics > storage for the entire database. If we have stats for a flexible > number of backends, it would need to be non-fixed, perhaps with the > entry for INVALID_PROC_NUMBER storing the global I/O stats, I > suppose. However, one concern with that approach would be the impact > on performance due to the frequent creation and deletion of stats > entries caused by high turnover of backends. As an additional benefit of this approach, the client can set a connection variable, for example, no_backend_iostats to true, or set its inverse variable to false, to restrict memory usage to only the required backends. regards. -- Kyotaro Horiguchi NTT Open Source Software Center ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: per backend I/O statistics @ 2024-09-04 04:45 Bertrand Drouvot <[email protected]> parent: Kyotaro Horiguchi <[email protected]> 0 siblings, 1 reply; 6+ messages in thread From: Bertrand Drouvot @ 2024-09-04 04:45 UTC (permalink / raw) To: Kyotaro Horiguchi <[email protected]>; +Cc: [email protected] Hi, On Tue, Sep 03, 2024 at 04:07:58PM +0900, Kyotaro Horiguchi wrote: > At Tue, 03 Sep 2024 15:37:49 +0900 (JST), Kyotaro Horiguchi <[email protected]> wrote in > > When I first looked at this patch, my initial thought was whether we > > should let these stats stay "fixed." The reason why the current > > PGSTAT_KIND_IO is fixed is that there is only one global statistics > > storage for the entire database. If we have stats for a flexible > > number of backends, it would need to be non-fixed, perhaps with the > > entry for INVALID_PROC_NUMBER storing the global I/O stats, I > > suppose. However, one concern with that approach would be the impact > > on performance due to the frequent creation and deletion of stats > > entries caused by high turnover of backends. > > As an additional benefit of this approach, the client can set a > connection variable, for example, no_backend_iostats to true, or set > its inverse variable to false, to restrict memory usage to only the > required backends. Thanks for the feedback! If we were to add an on/off switch button, I think I'd vote for a global one instead. Indeed, I see this feature more like an "Administrator" one, where the administrator wants to be able to find out which session is reponsible of what (from an I/O point of view): like being able to anwser "which session is generating this massive amount of reads"? If we allow each session to disable the feature then the administrator would lost this ability. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: per backend I/O statistics @ 2024-09-20 03:53 Michael Paquier <[email protected]> parent: Bertrand Drouvot <[email protected]> 0 siblings, 1 reply; 6+ messages in thread From: Michael Paquier @ 2024-09-20 03:53 UTC (permalink / raw) To: Bertrand Drouvot <[email protected]>; +Cc: Kyotaro Horiguchi <[email protected]>; [email protected] On Wed, Sep 04, 2024 at 04:45:24AM +0000, Bertrand Drouvot wrote: > On Tue, Sep 03, 2024 at 04:07:58PM +0900, Kyotaro Horiguchi wrote: >> As an additional benefit of this approach, the client can set a >> connection variable, for example, no_backend_iostats to true, or set >> its inverse variable to false, to restrict memory usage to only the >> required backends. > > Thanks for the feedback! > > If we were to add an on/off switch button, I think I'd vote for a global one > instead. Indeed, I see this feature more like an "Administrator" one, where > the administrator wants to be able to find out which session is reponsible of > what (from an I/O point of view): like being able to anwser "which session is > generating this massive amount of reads"? > > If we allow each session to disable the feature then the administrator > would lost this ability. Hmm, I've been studying this patch, and I am not completely sure to agree with this feeling of using fixed-numbered stats, actually, after reading the whole and seeing the structure of the patch (FLEXIBLE_ARRAY_MEMBER is a new way to handle the fact that we don't know exactly the number of slots we need to know for the fixed-numbered stats as MaxBackends may change). If we make these kind of stats variable-numbered, does it have to actually involve many creations or removals of the stats entries, though? One point is that the number of entries to know about is capped by max_connections, which is a PGC_POSTMASTER. That's the same kind of control as replication slots. So one approach would be to reuse entries in the dshash and use in the hashing key the number in the procarrays. If a new connection spawns and reuses a slot that was used in the past, then reset all the existing fields and assign its PID. Another thing is the consistency of the data that we'd like to keep at shutdown. If the connections have a balanced amount of stats shared among them, doing decision-making based on them is kind of easy. But that may cause confusion if the activity is unbalanced across the sessions. We could also not flush them to disk as an option, but it still seems more useful to me to save this data across restarts if one takes frequent snapshots of the new system view reporting everything, so as it is possible to get an idea of the deltas across the snapshots for each connection slot. -- Michael Attachments: [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc) download ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: per backend I/O statistics @ 2024-10-07 09:52 Bertrand Drouvot <[email protected]> parent: Michael Paquier <[email protected]> 0 siblings, 0 replies; 6+ messages in thread From: Bertrand Drouvot @ 2024-10-07 09:52 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Kyotaro Horiguchi <[email protected]>; [email protected] Hi, On Fri, Sep 20, 2024 at 12:53:43PM +0900, Michael Paquier wrote: > On Wed, Sep 04, 2024 at 04:45:24AM +0000, Bertrand Drouvot wrote: > > On Tue, Sep 03, 2024 at 04:07:58PM +0900, Kyotaro Horiguchi wrote: > >> As an additional benefit of this approach, the client can set a > >> connection variable, for example, no_backend_iostats to true, or set > >> its inverse variable to false, to restrict memory usage to only the > >> required backends. > > > > Thanks for the feedback! > > > > If we were to add an on/off switch button, I think I'd vote for a global one > > instead. Indeed, I see this feature more like an "Administrator" one, where > > the administrator wants to be able to find out which session is reponsible of > > what (from an I/O point of view): like being able to anwser "which session is > > generating this massive amount of reads"? > > > > If we allow each session to disable the feature then the administrator > > would lost this ability. > > Hmm, I've been studying this patch, Thanks for looking at it! > and I am not completely sure to > agree with this feeling of using fixed-numbered stats, actually, after > reading the whole and seeing the structure of the patch > (FLEXIBLE_ARRAY_MEMBER is a new way to handle the fact that we don't > know exactly the number of slots we need to know for the > fixed-numbered stats as MaxBackends may change). Right, that's a new way of dealing with "unknown" number of slots (and it has cons as you mentioned in [1]). > If we make these > kind of stats variable-numbered, does it have to actually involve many > creations or removals of the stats entries, though? One point is that > the number of entries to know about is capped by max_connections, > which is a PGC_POSTMASTER. That's the same kind of control as > replication slots. So one approach would be to reuse entries in the > dshash and use in the hashing key the number in the procarrays. If a > new connection spawns and reuses a slot that was used in the past, > then reset all the existing fields and assign its PID. Yeah, like it's done currently with the "fixed-numbered" stats proposal. That sounds reasonable to me, I'll look at this proposed approach and come back with a new patch version, thanks! > Another thing is the consistency of the data that we'd like to keep at > shutdown. If the connections have a balanced amount of stats shared > among them, doing decision-making based on them is kind of easy. But > that may cause confusion if the activity is unbalanced across the > sessions. We could also not flush them to disk as an option, but it > still seems more useful to me to save this data across restarts if one > takes frequent snapshots of the new system view reporting everything, > so as it is possible to get an idea of the deltas across the snapshots > for each connection slot. The idea that has been implemented so far in this patch is that we still maintain an aggregated version of the stats (visible through pg_stat_io) and that only the aggregated stats are flushed/read to/from disk (means we don't flush the per-backend stats). I think that it makes sense that way. The way I see it is that the per-backend I/O stats is more for current activity instrumentation. So it's not clear to me what would be the benefits of restoring the per-backend stats at startup knowing that: 1) we restored the aggregated stats and 2) the sessions that were responsible for the the restored stats are gone. [1]: https://www.postgresql.org/message-id/Zuz5iQ4AjcuOMx_w%40paquier.xyz Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com ^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2024-10-07 09:52 UTC | newest] Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2022-01-23 17:47 [PATCH v9 2/3] test Michail Nikolaev <[email protected]> 2024-09-03 06:37 Re: per backend I/O statistics Kyotaro Horiguchi <[email protected]> 2024-09-03 07:07 ` Re: per backend I/O statistics Kyotaro Horiguchi <[email protected]> 2024-09-04 04:45 ` Re: per backend I/O statistics Bertrand Drouvot <[email protected]> 2024-09-20 03:53 ` Re: per backend I/O statistics Michael Paquier <[email protected]> 2024-10-07 09:52 ` Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox