On Fri, Feb 28, 2025 at 2:29 PM Thomas Munro wrote:
> On Fri, Feb 28, 2025 at 11:58 AM Melanie Plageman
> wrote:
> > On Thu, Feb 27, 2025 at 1:08 PM Tom Lane wrote:
> > > I wonder if it'd be a good idea to add something like
> > >
> > > Assert(stream->distance == 1);
> > >
On Fri, Feb 28, 2025 at 11:58 AM Melanie Plageman
wrote:
> On Thu, Feb 27, 2025 at 1:08 PM Tom Lane wrote:
> > I wonder if it'd be a good idea to add something like
> >
> > Assert(stream->distance == 1);
> > Assert(stream->pending_read_nblocks == 0);
> >
On Thu, Feb 27, 2025 at 1:08 PM Tom Lane wrote:
>
> I wonder if it'd be a good idea to add something like
>
> Assert(stream->distance == 1);
> Assert(stream->pending_read_nblocks == 0);
> Assert(stream->per_buffer_data_size == 0);
> + A
Andres Freund writes:
> Ah, no, it isn't. But I still think the coverity alert and the patch don't
> make sense, as per the below:
Coverity's alert makes perfect sense if you posit that Coverity
doesn't assume that this read_stream_next_buffer call will
only be applied to a stream that has per_bu
Em qui., 27 de fev. de 2025 às 14:49, Andres Freund
escreveu:
> Hi,
>
> On 2025-02-27 12:44:24 -0500, Andres Freund wrote:
> > > CID 1592454: (#1 of 1): Explicit null dereferenced (FORWARD_NULL)
> > > 8. var_deref_op: Dereferencing null pointer per_buffer_data.
> >
> > That's exactly what the mes
Hi,
On 2025-02-27 12:44:24 -0500, Andres Freund wrote:
> > CID 1592454: (#1 of 1): Explicit null dereferenced (FORWARD_NULL)
> > 8. var_deref_op: Dereferencing null pointer per_buffer_data.
>
> That's exactly what the messages you quoted are discussing, no?
Ah, no, it isn't. But I still think th
Hi,
On 2025-02-27 14:32:28 -0300, Ranier Vilela wrote:
> Hi.
>
> Em ter., 18 de fev. de 2025 às 11:31, Melanie Plageman <
> melanieplage...@gmail.com> escreveu:
>
> > On Sun, Feb 16, 2025 at 1:12 PM Tom Lane wrote:
> > >
> > > Thomas Munro writes:
> > > > Thanks! It's green again.
> > >
> > >
Hi.
Em ter., 18 de fev. de 2025 às 11:31, Melanie Plageman <
melanieplage...@gmail.com> escreveu:
> On Sun, Feb 16, 2025 at 1:12 PM Tom Lane wrote:
> >
> > Thomas Munro writes:
> > > Thanks! It's green again.
> >
> > The security team's Coverity instance complained about this patch:
> >
> > **
Melanie Plageman writes:
> On Sun, Feb 16, 2025 at 1:12 PM Tom Lane wrote:
>> Basically, Coverity doesn't understand that a successful call to
>> read_stream_next_buffer must set per_buffer_data here. I don't
>> think there's much chance of teaching it that, so we'll just
>> have to dismiss this
On Sun, Feb 16, 2025 at 1:12 PM Tom Lane wrote:
>
> Thomas Munro writes:
> > Thanks! It's green again.
>
> The security team's Coverity instance complained about this patch:
>
> *** CID 1642971: Null pointer dereferences (FORWARD_NULL)
> /srv/coverity/git/pgsql-git/postgresql/src/backend/acces
Thomas Munro writes:
> Thanks! It's green again.
The security team's Coverity instance complained about this patch:
*** CID 1642971: Null pointer dereferences (FORWARD_NULL)
/srv/coverity/git/pgsql-git/postgresql/src/backend/access/heap/vacuumlazy.c:
1295 in lazy_scan_heap()
1289
On Sat, Feb 15, 2025 at 12:50 PM Melanie Plageman
wrote:
> It fixed the issue (after an off-list correction to the patch by Thomas).
Thanks! It's green again.
On Fri, Feb 14, 2025 at 6:09 PM Thomas Munro wrote:
>
> > Agreed that right now is a bad time to push this to v17 --- we need to
> > keep the risk factors as low as possible for the re-release. Master
> > now and v17 after the re-wrap seems like the right compromise.
>
> Cool, will push to master
Thomas Munro writes:
> Here's a patch. Is there a tidier way to write this?
Hmm, I think not with the current set of primitives. We could think
about refactoring them, but that's not a job for a band-aid patch.
> It should probably be back-patched to 17, because external code might
> use per-b
On Sat, Feb 15, 2025 at 12:03 PM Tom Lane wrote:
> Thomas Munro writes:
> > Here's a patch. Is there a tidier way to write this?
>
> Hmm, I think not with the current set of primitives. We could think
> about refactoring them, but that's not a job for a band-aid patch.
Thanks for looking.
> >
On Sat, Feb 15, 2025 at 10:50 AM Thomas Munro wrote:
> On Sat, Feb 15, 2025 at 7:30 AM Melanie Plageman
> wrote:
> > Seems valgrind doesn't like this [1]. I'm looking into it.
>
> Melanie was able to reproduce this on her local valgrind and
> eventually we figured out that it's my fault. I put c
On Sat, Feb 15, 2025 at 7:30 AM Melanie Plageman
wrote:
> Seems valgrind doesn't like this [1]. I'm looking into it.
Melanie was able to reproduce this on her local valgrind and
eventually we figured out that it's my fault. I put code into
read_stream.c that calls wipe_mem(), thinking that that
On Fri, Feb 14, 2025 at 1:15 PM Melanie Plageman
wrote:
>
> On Thu, Feb 13, 2025 at 9:06 PM Melanie Plageman
> wrote:
> >
> > On Thu, Feb 13, 2025 at 8:30 PM Masahiko Sawada
> > wrote:
> > > The rest looks good to me.
> >
> > Cool! I'll plan to push this tomorrow barring any objections.
>
> I'v
On Thu, Feb 13, 2025 at 9:06 PM Melanie Plageman
wrote:
>
> On Thu, Feb 13, 2025 at 8:30 PM Masahiko Sawada wrote:
> > The rest looks good to me.
>
> Cool! I'll plan to push this tomorrow barring any objections.
I've committed this and marked it as such in the CF app.
- Melanie
On Thu, Feb 13, 2025 at 8:30 PM Masahiko Sawada wrote:
>
> On Thu, Feb 13, 2025 at 4:55 PM Melanie Plageman
> wrote:
> >
> > We don't want to hang onto that pin for a
> > long time. But I can't move them to the bottom of the loop after we
> > release the buffer because some of the code paths don'
On Thu, Feb 13, 2025 at 4:55 PM Melanie Plageman
wrote:
>
> Thanks for your review! I've made the changes in attached v18.
>
> I do want to know what you think we should do about what you brought
> up about lazy_check_wraparound_failsafe() -- given my reply (below).
>
> On Thu, Feb 13, 2025 at 6:0
On Thu, Feb 13, 2025 at 6:52 PM Thomas Munro wrote:
>
> I've been poking, reading, and trying out these patches. They look good to
> me.
Thanks for the review.
> Tiny nit, maybe this comment could say something less obvious, cf the
> similar comment near the other stream:
>
> + /* Set up
Thanks for your review! I've made the changes in attached v18.
I do want to know what you think we should do about what you brought
up about lazy_check_wraparound_failsafe() -- given my reply (below).
On Thu, Feb 13, 2025 at 6:08 PM Masahiko Sawada wrote:
>
> Sorry for the late chiming in. I've
On Fri, Feb 14, 2025 at 12:11 PM Melanie Plageman
wrote:
> I've done some clean-up including incorporating a few off-list pieces
> of minor feedback from Andres.
I've been poking, reading, and trying out these patches. They look good to me.
Tiny nit, maybe this comment could say something less
On Tue, Feb 11, 2025 at 8:10 PM Melanie Plageman
wrote:
>
> On Thu, Feb 6, 2025 at 1:06 PM Melanie Plageman
> wrote:
> >
> > On Wed, Feb 5, 2025 at 5:26 PM Melanie Plageman
> > wrote:
> > >
> > > Yes, looking at these results, I also feel good about it. I've updated
> > > the commit metadata in
On Tue, Feb 11, 2025 at 5:10 PM Melanie Plageman
wrote:
>
> On Thu, Feb 6, 2025 at 1:06 PM Melanie Plageman
> wrote:
> >
> > On Wed, Feb 5, 2025 at 5:26 PM Melanie Plageman
> > wrote:
> > >
> > > Yes, looking at these results, I also feel good about it. I've updated
> > > the commit metadata in
On Thu, Feb 6, 2025 at 1:06 PM Melanie Plageman
wrote:
>
> On Wed, Feb 5, 2025 at 5:26 PM Melanie Plageman
> wrote:
> >
> > Yes, looking at these results, I also feel good about it. I've updated
> > the commit metadata in attached v14, but I could use a round of review
> > before pushing it.
>
>
On Wed, Feb 5, 2025 at 5:26 PM Melanie Plageman
wrote:
>
> Yes, looking at these results, I also feel good about it. I've updated
> the commit metadata in attached v14, but I could use a round of review
> before pushing it.
I've done a bit of self-review and updated these patches.
- Melanie
From
On Sat, Jan 18, 2025 at 11:51 AM Tomas Vondra wrote:
>
> Sure. I repeated the benchmark with v13, and it seems the behavior did
> change. I no longer see the "big" regression when most of the pages get
> updated (and need vacuuming).
>
> I can't be 100% sure this is due to changes in the patch, be
On 1/18/25 22:31, Thomas Munro wrote:
> On Sun, Jan 19, 2025 at 5:51 AM Tomas Vondra wrote:
>> * Does it still make sense to default to eic=1? For this particular test
>> increasing eic=4 often cuts the duration in half (especially on nvme
>> storage).
>
> Maybe it wasn't a bad choice for systems
On Sun, Jan 19, 2025 at 10:31 AM Thomas Munro wrote:
> read_stream.c doesn't do that
> sort of multiplication itself,
Actually for completeness there is a place where it allocates local
memory for max I/Os * 4, and that 4 is a not entirely unbogus and
should change to io_combine_limit for the AIO
On Sun, Jan 19, 2025 at 5:51 AM Tomas Vondra wrote:
> * Does it still make sense to default to eic=1? For this particular test
> increasing eic=4 often cuts the duration in half (especially on nvme
> storage).
Maybe it wasn't a bad choice for systems with one spinning disk, but
obviously typical
On Sun, Dec 15, 2024 at 10:10 AM Tomas Vondra wrote:
>
> I've been looking at some other vacuum-related patches, so I took a look
> at these remaining bits too. I don't have much to say about the code
> (seems perfectly fine to me), so I decided to do a bit of testing.
Thanks for doing this!
> I
On Tue, Jul 16, 2024 at 1:52 PM Noah Misch wrote:
> On Mon, Jul 15, 2024 at 03:26:32PM +1200, Thomas Munro wrote:
> That's reasonable. radixtree already forbids mutations concurrent with
> iteration, so there's no new concurrency hazard. One alternative is
> per_buffer_data big enough for MaxOff
On Mon, Jul 15, 2024 at 03:26:32PM +1200, Thomas Munro wrote:
> On Mon, Jul 8, 2024 at 2:49 AM Noah Misch wrote:
> > what is the scope of the review you seek?
>
> The patch "Refactor tidstore.c memory management." could definitely
> use some review.
That's reasonable. radixtree already forbids
On Mon, Jul 8, 2024 at 2:49 AM Noah Misch wrote:
> what is the scope of the review you seek?
The patch "Refactor tidstore.c memory management." could definitely
use some review. I wasn't sure if that should be proposed in a new
thread of its own, but then the need for it comes from this
streamif
On Sun, Jul 7, 2024 at 10:49 AM Noah Misch wrote:
>
> On Fri, Jun 28, 2024 at 05:36:25PM -0400, Melanie Plageman wrote:
> > I've attached a WIP v11 streaming vacuum patch set here that is
> > rebased over master (by Thomas), so that I could add a CF entry for
> > it. It still has the problem with
On Fri, Jun 28, 2024 at 05:36:25PM -0400, Melanie Plageman wrote:
> I've attached a WIP v11 streaming vacuum patch set here that is
> rebased over master (by Thomas), so that I could add a CF entry for
> it. It still has the problem with the extra WAL write and fsync calls
> investigated by Thomas
On Sun, Mar 17, 2024 at 2:53 AM Thomas Munro wrote:
>
> 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 t
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,
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 AP
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!
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
+++ b/src/back
On Mon, Mar 11, 2024 at 11:29:44AM +0200, Heikki Linnakangas wrote:
>
> > I see why you removed my treatise-level comment that was here about
> > unskipped skippable blocks. However, when I was trying to understand
> > this code, I did wish there was some comment that explained to me why we
> > nee
On 08/03/2024 19:34, 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 +0200, Heikki Linnakangas wrote:
H
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
> >
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 address
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 +0200,
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 +0200, Heikki Linnakangas wrote:
> > I will say that now all of the varia
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(),
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 DISABLE_PAGE_SKIPPI
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 +0200, Heikki Linnakangas wrote:
I will say that now all of the variable names are *very* long. I didn't
want to remove the "state" from LVRelState->
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
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 true
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
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 comments,
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 par
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 f
On Wed, Mar 06, 2024 at 10:00:23PM -0500, Melanie Plageman wrote:
> On Wed, Mar 06, 2024 at 09:55:21PM +0200, Heikki Linnakangas wrote:
> > I made some further changes. I kept them as separate commits for easier
> > review, see the commit messages for details. Any thoughts on those changes?
>
> I'
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 pa
On Tue, Feb 27, 2024 at 02:47:03PM -0500, Melanie Plageman wrote:
> On Mon, Jan 29, 2024 at 8:18 PM Melanie Plageman
> wrote:
> >
> > On Fri, Jan 26, 2024 at 8:28 AM vignesh C wrote:
> > >
> > > CFBot shows that the patch does not apply anymore as in [1]:
> > > === applying patch
> > > ./v3-0002-
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 patches for now. I like those
changes a lot for the sake of readablity even without any
On Fri, Jan 26, 2024 at 8:28 AM vignesh C wrote:
>
> CFBot shows that the patch does not apply anymore as in [1]:
> === applying patch
> ./v3-0002-Add-lazy_scan_skip-unskippable-state-to-LVRelStat.patch
> patching file src/backend/access/heap/vacuumlazy.c
> ...
> Hunk #10 FAILED at 1042.
> Hunk #1
On Fri, 12 Jan 2024 at 05:12, Melanie Plageman
wrote:
>
> v3 attached
>
> On Thu, Jan 4, 2024 at 3:23 PM Andres Freund wrote:
> >
> > Hi,
> >
> > 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
> > >
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 12:36
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 unn
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
> >
v3 attached
On Thu, Jan 4, 2024 at 3:23 PM Andres Freund wrote:
>
> Hi,
>
> 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
> > nskipp
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
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 m
Hi,
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 may lead to worse code - the compiler has to reload
vacre
On Sun, Dec 31, 2023 at 1:28 PM Melanie Plageman
wrote:
>
> There are a few comments that still need to be updated. I also noticed I
> needed to reorder and combine a couple of the commits. I wanted to
> register this for the january commitfest, so I didn't quite have time
> for the finishing touc
72 matches
Mail list logo
