Re: Confine vacuum skip logic to lazy_scan_skip

2024-03-17 Thread Thomas Munro
On Tue, Mar 12, 2024 at 10:03 AM Melanie Plageman wrote: > I've rebased the attached v10 over top of the changes to > lazy_scan_heap() Heikki just committed and over the v6 streaming read > patch set. I started testing them and see that you are right, we no > longer pin too many buffers. However,

Re: Confine vacuum skip logic to lazy_scan_skip

2024-03-11 Thread Melanie Plageman
On Sun, Mar 10, 2024 at 11:01 PM Thomas Munro wrote: > > On Mon, Mar 11, 2024 at 5:31 AM Melanie Plageman > wrote: > > I have investigated the interaction between > > maintenance_io_concurrency, streaming reads, and the vacuum buffer > > access strategy (BAS_VACUUM). > > > > The streaming read

Re: Confine vacuum skip logic to lazy_scan_skip

2024-03-11 Thread Melanie Plageman
On Mon, Mar 11, 2024 at 2:47 PM Heikki Linnakangas wrote: > > > > Otherwise LGTM > > Ok, pushed! Thank you, this is much more understandable now! Cool, thanks!

Re: Confine vacuum skip logic to lazy_scan_skip

2024-03-11 Thread Heikki Linnakangas
On 11/03/2024 18:15, Melanie Plageman wrote: On Mon, Mar 11, 2024 at 11:29:44AM +0200, Heikki Linnakangas wrote: diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index ac55ebd2ae..1757eb49b7 100644 --- a/src/backend/access/heap/vacuumlazy.c +++

Re: Confine vacuum skip logic to lazy_scan_skip

2024-03-11 Thread Melanie Plageman
00:00 2001 > From: Heikki Linnakangas > Date: Fri, 8 Mar 2024 17:32:19 +0200 > Subject: [PATCH v9 1/2] Confine vacuum skip logic to lazy_scan_skip() > > Rename lazy_scan_skip() to heap_vac_scan_next_block() and move more > code into the function, so that the caller doesn't need t

Re: Confine vacuum skip logic to lazy_scan_skip

2024-03-11 Thread Heikki Linnakangas
ngas Date: Fri, 8 Mar 2024 17:32:19 +0200 Subject: [PATCH v9 1/2] Confine vacuum skip logic to lazy_scan_skip() Rename lazy_scan_skip() to heap_vac_scan_next_block() and move more code into the function, so that the caller doesn't need to know about ranges or skipping anymore. heap_vac_scan_

Re: Confine vacuum skip logic to lazy_scan_skip

2024-03-10 Thread Thomas Munro
On Mon, Mar 11, 2024 at 5:31 AM Melanie Plageman wrote: > On Wed, Mar 6, 2024 at 6:47 PM Melanie Plageman > wrote: > > Performance results: > > > > The TL;DR of my performance results is that streaming read vacuum is > > faster. However there is an issue with the interaction of the streaming > >

Re: Confine vacuum skip logic to lazy_scan_skip

2024-03-10 Thread Melanie Plageman
On Wed, Mar 6, 2024 at 6:47 PM Melanie Plageman wrote: > > Performance results: > > The TL;DR of my performance results is that streaming read vacuum is > faster. However there is an issue with the interaction of the streaming > read code and the vacuum buffer access strategy which must be

Re: Confine vacuum skip logic to lazy_scan_skip

2024-03-08 Thread Melanie Plageman
On Fri, Mar 8, 2024 at 12:34 PM Melanie Plageman wrote: > > On Fri, Mar 08, 2024 at 06:07:33PM +0200, Heikki Linnakangas wrote: > > On 08/03/2024 02:46, Melanie Plageman wrote: > > > On Wed, Mar 06, 2024 at 10:00:23PM -0500, Melanie Plageman wrote: > > > > On Wed, Mar 06, 2024 at 09:55:21PM

Re: Confine vacuum skip logic to lazy_scan_skip

2024-03-08 Thread Melanie Plageman
> heap pages were dirtied, even if there were no changes. We would also > fail to clear any VM bits that were set incorrectly. > > This was broken in commit 980ae17310, so backpatch to v16. LGTM. > From 47af1ca65cf55ca876869b43bff47f9d43f0750e Mon Sep 17 00:00:00 2001 > From: Heikki Linnakangas > Date: Fri, 8 M

Re: Confine vacuum skip logic to lazy_scan_skip

2024-03-08 Thread Melanie Plageman
On Fri, Mar 8, 2024 at 11:31 AM Melanie Plageman wrote: > > On Fri, Mar 8, 2024 at 11:00 AM Peter Geoghegan wrote: > > > > On Fri, Mar 8, 2024 at 10:48 AM Melanie Plageman > > wrote: > > > Not that it will be fun to maintain another special case in the VM > > > update code in lazy_scan_prune(),

Re: Confine vacuum skip logic to lazy_scan_skip

2024-03-08 Thread Melanie Plageman
On Fri, Mar 8, 2024 at 11:00 AM Peter Geoghegan wrote: > > On Fri, Mar 8, 2024 at 10:48 AM Melanie Plageman > wrote: > > Not that it will be fun to maintain another special case in the VM > > update code in lazy_scan_prune(), but we could have a special case > > that checks if

Re: Confine vacuum skip logic to lazy_scan_skip

2024-03-08 Thread Heikki Linnakangas
From: Heikki Linnakangas Date: Fri, 8 Mar 2024 17:32:19 +0200 Subject: [PATCH v8 2/3] Confine vacuum skip logic to lazy_scan_skip() Rename lazy_scan_skip() to heap_vac_scan_next_block() and move more code into the function, so that the caller doesn't need to know about ranges or skipping an

Re: Confine vacuum skip logic to lazy_scan_skip

2024-03-08 Thread Peter Geoghegan
On Fri, Mar 8, 2024 at 11:00 AM Peter Geoghegan wrote: > Seems like it might be possible to simplify/consolidate the VM-setting > code that's now located at the end of lazy_scan_prune. Perhaps the two > distinct blocks that call visibilitymap_set() could be combined into > one. FWIW I think that

Re: Confine vacuum skip logic to lazy_scan_skip

2024-03-08 Thread Peter Geoghegan
On Fri, Mar 8, 2024 at 10:48 AM Melanie Plageman wrote: > Not that it will be fun to maintain another special case in the VM > update code in lazy_scan_prune(), but we could have a special case > that checks if DISABLE_PAGE_SKIPPING was passed to vacuum and if > all_visible_according_to_vm is

Re: Confine vacuum skip logic to lazy_scan_skip

2024-03-08 Thread Melanie Plageman
On Fri, Mar 8, 2024 at 10:41 AM Peter Geoghegan wrote: > > On Fri, Mar 8, 2024 at 8:49 AM Heikki Linnakangas wrote: > > ISTM we should revert the above hunk, and backpatch it to v16. I'm a > > little wary because I don't understand why that change was made in the > > first place, though. I think

Re: Confine vacuum skip logic to lazy_scan_skip

2024-03-08 Thread Melanie Plageman
On Fri, Mar 8, 2024 at 8:49 AM Heikki Linnakangas wrote: > > On 08/03/2024 02:46, Melanie Plageman wrote: > > On Wed, Mar 06, 2024 at 10:00:23PM -0500, Melanie Plageman wrote: > >>> I feel heap_vac_scan_get_next_block() function could use some love. Maybe > >>> just some rewording of the

Re: Confine vacuum skip logic to lazy_scan_skip

2024-03-08 Thread Peter Geoghegan
On Fri, Mar 8, 2024 at 8:49 AM Heikki Linnakangas wrote: > ISTM we should revert the above hunk, and backpatch it to v16. I'm a > little wary because I don't understand why that change was made in the > first place, though. I think it was just an ill-advised attempt at > tidying up the code as

Re: Confine vacuum skip logic to lazy_scan_skip

2024-03-08 Thread Heikki Linnakangas
On 08/03/2024 02:46, Melanie Plageman wrote: On Wed, Mar 06, 2024 at 10:00:23PM -0500, Melanie Plageman wrote: I feel heap_vac_scan_get_next_block() function could use some love. Maybe just some rewording of the comments, or maybe some other refactoring; not sure. But I'm pretty happy with the

Re: Confine vacuum skip logic to lazy_scan_skip

2024-03-07 Thread Melanie Plageman
GES_THRESHOLD) + vacrel->next_block_state.skipping_current_range = false; else { - *skipping_current_range = true; + vacrel->next_block_state.skipping_current_range = true; if (skipsallvis) vacrel->skippedallvis = true; } - return next_unskippable_block; + if (next_unskippabl

Re: Confine vacuum skip logic to lazy_scan_skip

2024-03-06 Thread Melanie Plageman
On Wed, Mar 06, 2024 at 09:55:21PM +0200, Heikki Linnakangas wrote: > On 27/02/2024 21:47, Melanie Plageman wrote: > > The attached v5 has some simplifications when compared to v4 but takes > > largely the same approach. > > > > 0001-0004 are refactoring > > I'm looking at just these 0001-0004

Re: Confine vacuum skip logic to lazy_scan_skip

2024-03-06 Thread Melanie Plageman
tate *vacrel, * true, so we must check both all_visible and all_frozen. */ else if (all_visible_according_to_vm && all_visible && - all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, )) + all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, >skip.vmbuffer)) { /* * Avoi

Re: Confine vacuum skip logic to lazy_scan_skip

2024-03-06 Thread Heikki Linnakangas
prune(LVRelState *vacrel, */ Assert(!TransactionIdIsValid(visibility_cutoff_xid)); visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr, - vmbuffer, InvalidTransactionId, + vacrel->skip.vmbuffer, InvalidTransactionId, VISIBILITYMAP_ALL_VISIBLE | VISIBILITYMAP_AL

Re: Confine vacuum skip logic to lazy_scan_skip

2024-01-29 Thread Melanie Plageman
57030c Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Sat, 30 Dec 2023 16:59:27 -0500 Subject: [PATCH v4 3/4] Confine vacuum skip logic to lazy_scan_skip In preparation for vacuum to use the streaming read interface (and eventually AIO), refactor vacuum's logic for skipping blocks such

Re: Confine vacuum skip logic to lazy_scan_skip

2024-01-26 Thread vignesh C
ake those commits more clear, first > > > introduce the struct, VacSkipState, which will maintain the variables > > > needed to skip ranges less than SKIP_PAGES_THRESHOLD. > > > > Why not add this to LVRelState, possibly as a struct embedded within it? > > Done in

Re: Confine vacuum skip logic to lazy_scan_skip

2024-01-12 Thread Melanie Plageman
On Fri, Jan 12, 2024 at 2:02 PM Jim Nasby wrote: > > On 1/11/24 5:50 PM, Melanie Plageman wrote: > > On Fri, Jan 5, 2024 at 5:51 AM Nazir Bilal Yavuz wrote: > >> > >> On Fri, 5 Jan 2024 at 02:25, Jim Nasby wrote: > >>> > >>> On 1/4/24 2:23 PM, Andres Freund wrote: > >>> > >>> On 2024-01-02

Re: Confine vacuum skip logic to lazy_scan_skip

2024-01-12 Thread Jim Nasby
On 1/11/24 5:50 PM, Melanie Plageman wrote: On Fri, Jan 5, 2024 at 5:51 AM Nazir Bilal Yavuz wrote: On Fri, 5 Jan 2024 at 02:25, Jim Nasby wrote: On 1/4/24 2:23 PM, Andres Freund wrote: On 2024-01-02 12:36:18 -0500, Melanie Plageman wrote: Subject: [PATCH v2 1/6] lazy_scan_skip remove

Re: Confine vacuum skip logic to lazy_scan_skip

2024-01-11 Thread Melanie Plageman
On Fri, Jan 5, 2024 at 5:51 AM Nazir Bilal Yavuz wrote: > > On Fri, 5 Jan 2024 at 02:25, Jim Nasby wrote: > > > > On 1/4/24 2:23 PM, Andres Freund wrote: > > > > On 2024-01-02 12:36:18 -0500, Melanie Plageman wrote: > > > > Subject: [PATCH v2 1/6] lazy_scan_skip remove unnecessary local var > >

Re: Confine vacuum skip logic to lazy_scan_skip

2024-01-11 Thread Melanie Plageman
ot add this to LVRelState, possibly as a struct embedded within it? Done in attached. > > From 335faad5948b2bec3b83c2db809bb9161d373dcb Mon Sep 17 00:00:00 2001 > > From: Melanie Plageman > > Date: Sat, 30 Dec 2023 16:59:27 -0500 > > Subject: [PATCH v2 4/6] Confine vacuum skip logic to lazy_scan_skip > >

Re: Confine vacuum skip logic to lazy_scan_skip

2024-01-05 Thread Nazir Bilal Yavuz
Hi, On Fri, 5 Jan 2024 at 02:25, Jim Nasby wrote: > > On 1/4/24 2:23 PM, Andres Freund wrote: > > On 2024-01-02 12:36:18 -0500, Melanie Plageman wrote: > > Subject: [PATCH v2 1/6] lazy_scan_skip remove unnecessary local var rel_pages > Subject: [PATCH v2 2/6] lazy_scan_skip remove unneeded local

Re: Confine vacuum skip logic to lazy_scan_skip

2024-01-04 Thread Jim Nasby
On 1/4/24 2:23 PM, Andres Freund wrote: On 2024-01-02 12:36:18 -0500, Melanie Plageman wrote: Subject: [PATCH v2 1/6] lazy_scan_skip remove unnecessary local var rel_pages Subject: [PATCH v2 2/6] lazy_scan_skip remove unneeded local var nskippable_blocks I think these

Re: Confine vacuum skip logic to lazy_scan_skip

2024-01-04 Thread Andres Freund
ly as a struct embedded within it? > From 335faad5948b2bec3b83c2db809bb9161d373dcb Mon Sep 17 00:00:00 2001 > From: Melanie Plageman > Date: Sat, 30 Dec 2023 16:59:27 -0500 > Subject: [PATCH v2 4/6] Confine vacuum skip logic to lazy_scan_skip > > In preparation for vacuum to use th

Re: Confine vacuum skip logic to lazy_scan_skip

2024-01-02 Thread Melanie Plageman
Confine vacuum skip logic to lazy_scan_skip In preparation for vacuum to use the streaming read interface (and eventually AIO), refactor vacuum's logic for skipping blocks such that it is entirely confined to lazy_scan_skip(). This turns lazy_scan_skip() and the VacSkipState it uses into an iterat

Confine vacuum skip logic to lazy_scan_skip

2023-12-31 Thread Melanie Plageman
e06f450dc7f60494751f78e00d43272 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Sat, 30 Dec 2023 16:59:27 -0500 Subject: [PATCH v1 4/7] Confine vacuum skip logic to lazy_scan_skip In preparation for vacuum to use the streaming read interface (and eventually AIO), refactor vacuum's logic for ski