Re: Use pgBufferUsage for block reporting in analyze

2024-05-28 Thread Anthonin Bonnefoy
Thanks for having a look. On Fri, May 10, 2024 at 12:40 PM Michael Paquier wrote: > This needs some runtime check to make sure that the calculations > are consistent before and after the fact (cannot do that now). > Yeah, testing this is also a bit painful as buffer usage of analyze is only

Possible incorrect row estimation for Gather paths

2024-05-24 Thread Anthonin Bonnefoy
Hi, While experimenting on an explain option to display all plan candidates (very rough prototype here [1]), I've noticed some discrepancies in some generated plans. EXPLAIN (ALL_CANDIDATES) SELECT * FROM pgbench_accounts order by aid; Plan 1 -> Gather Merge (cost=11108.32..22505.38

Use pgBufferUsage for block reporting in analyze

2024-05-10 Thread Anthonin Bonnefoy
Hi, Analyze logs within autovacuum uses specific variables VacuumPage{Hit,Miss,Dirty} to track the buffer usage count. However, pgBufferUsage already provides block usage tracking and handles more cases (temporary tables, parallel workers...). Those variables were only used in two places, block

Re: Fix parallel vacuum buffer usage reporting

2024-05-03 Thread Anthonin Bonnefoy
On Wed, May 1, 2024 at 5:37 AM Masahiko Sawada wrote: > Thank you for further testing! I've pushed the patch. Thanks! Here is the rebased version for the follow-up patch removing VacuumPage variables. Though I'm not sure if I should create a dedicated mail thread since the bug was fixed and

Re: Fix parallel vacuum buffer usage reporting

2024-04-30 Thread Anthonin Bonnefoy
I've done some additional tests to validate the reported numbers. Using pg_statio, it's possible to get the minimum number of block hits (Full script attached). -- Save block hits before vacuum SELECT pg_stat_force_next_flush(); SELECT heap_blks_hit, idx_blks_hit FROM pg_statio_all_tables where

Re: Fix parallel vacuum buffer usage reporting

2024-04-25 Thread Anthonin Bonnefoy
On Wed, Apr 24, 2024 at 4:01 PM Alena Rybakina wrote: > I tested the main postgres branch with and without your fix using a script > that was written by me. It consists of five scenarios and I made a > comparison in the logs between the original version of the master branch > and the master

Re: Fix parallel vacuum buffer usage reporting

2024-04-24 Thread Anthonin Bonnefoy
Thanks for the review! > I think that if the anayze command doesn't have the same issue, we > don't need to change it. Good point, I've wrongly assumed that analyze was also impacted but there's no parallel analyze so the block count is correct. > (a) make lazy vacuum use BufferUsage instead

Re: Fix parallel vacuum buffer usage reporting

2024-04-22 Thread Anthonin Bonnefoy
On Sat, Apr 20, 2024 at 2:00 PM Alena Rybakina wrote: > Hi, thank you for your work with this subject. > > While I was reviewing your code, I noticed that your patch conflicts with > another patch [0] that been committed. > > [0] >

Re: Fix parallel vacuum buffer usage reporting

2024-03-28 Thread Anthonin Bonnefoy
Hi, Thanks for the review. On Thu, Mar 28, 2024 at 4:07 AM Masahiko Sawada wrote: > As for the proposed patch, the following part should handle the temp > tables too: > True, I've missed the local blocks. I've updated the patch: - read_rate and write_rate now include local block usage - I've

Re: POC: Extension for adding distributed tracing - pg_tracing

2024-03-12 Thread Anthonin Bonnefoy
Hi all! Thanks for the reviews and comments. > - pg_tracing.c should include postgres.h as the first thing Will do. > afaict none of the use of volatile is required, spinlocks have been barriers > for a long time now Got it, I will remove them. I've been mimicking what was done in

Fix expecteddir argument in pg_regress

2024-03-11 Thread Anthonin Bonnefoy
Hi all! pg_regress accepts the expecteddir argument. However, it is never used and outputdir is used instead to get the expected files paths. This patch fixes this and uses the expecteddir argument as expected. Regards, Anthonin v01-0001-pg_regress-Use-expecteddir-for-the-expected-file.patch

Fix parallel vacuum buffer usage reporting

2024-02-09 Thread Anthonin Bonnefoy
Hi, With a db setup with pgbench, we add an additional index: CREATE INDEX ON pgbench_accounts(abalance) And trigger several updates and vacuum to reach a stable amount of dirtied pages: UPDATE pgbench_accounts set abalance = abalance + 1 WHERE aid=1; CHECKPOINT; VACUUM (VERBOSE, INDEX_CLEANUP

Re: POC: Extension for adding distributed tracing - pg_tracing

2024-02-06 Thread Anthonin Bonnefoy
Hi! On Fri, Jan 26, 2024 at 1:43 PM Nikita Malakhov wrote: > It's a good idea to split a big patch into several smaller ones. > But you've already implemented these features - you could provide them > as sequential small patches (i.e. v13-0002-guc-context-propagation.patch and > so on) Keeping

Re: A performance issue with Memoize

2024-01-31 Thread Anthonin Bonnefoy
Hi, I've seen a similar issue with the following query (tested on the current head): EXPLAIN ANALYZE SELECT * FROM tenk1 t1 LEFT JOIN LATERAL (SELECT t1.two, tenk2.hundred, tenk2.two FROM tenk2) t2 ON t1.hundred = t2.hundred WHERE t1.hundred < 5;

Re: Add \syncpipeline command to pgbench

2024-01-22 Thread Anthonin Bonnefoy
script is reached while there's still an ongoing pipeline. 0002 adds the \syncpipeline command (original patch with an additional test case). Regards, Anthonin On Mon, Jan 22, 2024 at 7:16 AM Michael Paquier wrote: > > On Fri, Jan 19, 2024 at 08:55:31AM +0100, Anthonin Bonnefoy wrote: &g

Re: Add \syncpipeline command to pgbench

2024-01-18 Thread Anthonin Bonnefoy
On Fri, Jan 19, 2024 at 5:08 AM Michael Paquier wrote: > The logic looks sound, but I have a > comment about the docs: could it be better to group \syncpipeline with > \startpipeline and \endpipeline? \syncpipeline requires a pipeline to > work. I've updated the doc to group the commands. It

Add \syncpipeline command to pgbench

2024-01-18 Thread Anthonin Bonnefoy
Hi, PQsendSyncMessage() was added in https://commitfest.postgresql.org/46/4262/. It allows users to add a Sync message without flushing the buffer. As a follow-up, this change adds an additional meta-command to pgbench, \syncpipeline, which will call PQsendSyncMessage(). This will make it

Re: [PATCH] Add additional extended protocol commands to psql: \parse and \bindx

2024-01-18 Thread Anthonin Bonnefoy
> Hmm. So it does not lead to any user-visible changes, right? >From what I can tell, there's no change in the behaviour. All paths would eventually go through HandleSlashCmds's cleaning logic. This is also mentioned in ignore_slash_options's comment. * Read and discard "normal" slash command

Re: [PATCH] Add additional extended protocol commands to psql: \parse and \bindx

2024-01-17 Thread Anthonin Bonnefoy
> I do realize the same is true for plain \bind, but it seems > like a bug there too. The unscanned bind's parameters are discarded later in the HandleSlashCmds functions. So adding the ignore_slash_options() for inactive branches scans and discards them earlier. I will add it to match what's

Re: [PATCH] Add additional extended protocol commands to psql: \parse and \bindx

2024-01-16 Thread Anthonin Bonnefoy
tiny nitpick: stmt_name should be replaced with STMT_NAME in > this line of the help message. Fixed On Sat, Jan 13, 2024 at 3:37 PM Jelte Fennema-Nio wrote: > > On Thu, 2 Nov 2023 at 10:52, Anthonin Bonnefoy > wrote: > > The main goal is to provide more ways to test extended pr

Re: POC: Extension for adding distributed tracing - pg_tracing

2024-01-04 Thread Anthonin Bonnefoy
Hi, > This approach avoids the need to rewrite SQL or give special meaning to SQL > comments. SQLCommenter already has a good amount of support and is referenced in opentelemetry https://github.com/open-telemetry/opentelemetry-sqlcommenter So the goal was more to leverage the existing trace

Re: Possible segfault when sending notification within a ProcessUtility hook

2023-12-06 Thread Anthonin Bonnefoy
> On Tue, Dec 5, 2023 at 9:03 PM Tom Lane wrote: > Why should we regard that as anything other than a bug in the > ProcessUtility hook? A failed transaction should not send any > notifies. Fair point. That was also my initial assumption but I thought that the transaction state was not available

Possible segfault when sending notification within a ProcessUtility hook

2023-12-05 Thread Anthonin Bonnefoy
Hi, I've encountered the following segfault: #0: 0x000104e821a8 postgres`list_head(l=0x7f7f7f7f7f7f7f7f) at pg_list.h:130:17 #1: 0x000104e81c9c postgres`PreCommit_Notify at async.c:932:16 #2: 0x000104dd02f8 postgres`CommitTransaction at xact.c:2236:2 #3: 0x000104dcfc24

Re: Add PQsendSyncMessage() to libpq

2023-11-13 Thread Anthonin Bonnefoy
Hi, I've played a bit with the patch on my side. One thing that would be great would be to make this available in pgbench through a \syncpipeline meta command. That would make it easier for users to test whether there's a positive impact with their queries or not. I've wrote a patch to add it to

[PATCH] Add additional extended protocol commands to psql: \parse and \bindx

2023-11-02 Thread Anthonin Bonnefoy
Hi all! Currently, only unnamed prepared statements are supported by psql with the \bind command and it's not possible to create or use named prepared statements through extended protocol. This patch introduces 2 additional commands: \parse and \bindx. \parse allows us to issue a Parse message

Re: POC: Extension for adding distributed tracing - pg_tracing

2023-07-28 Thread Anthonin Bonnefoy
> What do you think about using INSTR_TIME_SET_CURRENT, INSTR_TIME_SUBTRACT and INSTR_TIME_GET_MILLISEC > macros for timing calculations? If you're talking of the two instances where I'm modifying the instr_time's ticks, it's because I can't use the macros there. The first case is for the parse

Re: POC: Extension for adding distributed tracing - pg_tracing

2023-07-28 Thread Anthonin Bonnefoy
Hi, > I'd keep Autotools build up to date Definitely. The patch is still in a rough state and updating Autotools fell through the cracks. > I'm currently playing with the patch and > reviewing sources, if you need any cooperation - please let us know. The goal for me was to get validation on the

Re: POC: Extension for adding distributed tracing - pg_tracing

2023-07-28 Thread Anthonin Bonnefoy
> Agree, something goes wrong when using Autotools (but not Meson) on > both Linux and MacOS. I didn't investigate the issue though. I was only using meson and forgot to keep Automake files up to date when I've split pg_tracing.c in multiple files (span.c, explain.c...). On Fri, Jul 28, 2023 at

Re: POC: Extension for adding distributed tracing - pg_tracing

2023-07-26 Thread Anthonin Bonnefoy
I've initially thought of sending the spans from PostgreSQL since this is the usual behavior of tracing libraries. However, this created a lot potential issues: - Protocol support and differences between trace collectors. OpenTelemetry seems to use gRPC, others are using http and those will

Re: Flush SLRU counters in checkpointer process

2023-07-19 Thread Anthonin Bonnefoy
com/postgres/postgres/blob/c8e1ba736b2b9e8c98d37a5b77c4ed31baf94147/src/backend/utils/activity/pgstat_slru.c#L161-L162 ). I will try to get a new patch with improved test stability. On Mon, Jul 3, 2023 at 3:18 PM Daniel Gustafsson wrote: > > On 3 Mar 2023, at 09:06, Anthonin

[PATCH] Add statement_timeout in pg_stat_activity

2023-04-04 Thread Anthonin Bonnefoy
Hello hackers. This patch adds the backend's statement_timeout value to pg_stat_activity. This would provide some insights on clients that are disabling a default statement timeout or overriding it through a pgbouncer, messing with other sessions. pg_stat_activity seemed like the best place to

Re: Flush SLRU counters in checkpointer process

2023-03-03 Thread Anthonin Bonnefoy
Here's the patch rebased with Andres' suggestions. Happy to update it if there's any additionalj change required. On Wed, Mar 1, 2023 at 8:46 PM Gregory Stark (as CFM) wrote: > On Thu, 12 Jan 2023 at 03:46, Anthonin Bonnefoy > wrote: > > > > > > That would make sense.

Re: Flush SLRU counters in checkpointer process

2023-01-12 Thread Anthonin Bonnefoy
On Wed, Jan 11, 2023 at 5:33 PM Andres Freund wrote: > Hi, > > On 2023-01-11 10:29:06 +0100, Anthonin Bonnefoy wrote: > > Currently, the Checkpointer process only reports SLRU statistics at > server > > shutdown, leading to delayed statistics for SLRU flushes. This

Flush SLRU counters in checkpointer process

2023-01-11 Thread Anthonin Bonnefoy
Hello hackers, Currently, the Checkpointer process only reports SLRU statistics at server shutdown, leading to delayed statistics for SLRU flushes. This patch adds a flush of SLRU stats to the end of checkpoints. Best regards, Anthonin flush-slru-counters.patch Description: Binary data