Hi,
Thank you for looking into this!
On Sat, 29 Mar 2025 at 23:10, Melanie Plageman
wrote:
>
> On Fri, Nov 29, 2024 at 6:20 AM Nazir Bilal Yavuz wrote:
> >
> > v4 is attached.
>
> I've started looking at this version. What I don't like about this
> structure is that we are forced to increment t
On Tue, Apr 1, 2025 at 7:21 AM Nazir Bilal Yavuz wrote:
>
> On Tue, 1 Apr 2025 at 05:14, Melanie Plageman
> wrote:
> >
> +1 for using the functions. I think it is hard to follow / maintain
> this with the do-while loops and goto statements.
I'll take a look at your downthread proposal in a bit.
On Thu, Apr 3, 2025 at 4:22 PM Daniel Gustafsson wrote:
>
> >> I had a quick look at this. Looks good overall
>
> Same here, this seemed like a good piece to bite into with my limited AIO
> knowledge to learn more, and reading it over it seems like a good change.
Thanks for taking a look!
> A fe
On Mon, Mar 31, 2025 at 2:58 PM Nazir Bilal Yavuz wrote:
>
> Do you think that I should continue to
> attach both approaches?
No, for now let's try and get this approach to a good place and then
see which one we like.
I think there might be another problem with the code. We only set
cur_database
On Sun, Mar 30, 2025 at 10:01 AM Nazir Bilal Yavuz wrote:
>
> > Some review feedback on your v4: I don't think we need the
> > rs_have_free_buffer per_buffer_data. We can just check
> > have_free_buffers() in both the callback and main loop in
> > autoprewarm_database_main(). I also think you want
On Mon, Mar 31, 2025 at 12:27 PM Nazir Bilal Yavuz wrote:
>
> I worked on an alternative approach, I refactored code a bit. It does
> not traverse the list two times and I think the code is more suitable
> to use read streams now. I simply get how many blocks are processed by
> read streams and mo
On Mon, Mar 31, 2025 at 3:45 PM Melanie Plageman
wrote:
>
> Whoops, this isn't right. It does work. I'm going to draft a version
> suggesting slightly different variable naming and a couple comments to
> make this more clear.
Okay, so after further study, I think there are multiple issues still
w
On Wed, Apr 2, 2025 at 1:20 PM Nazir Bilal Yavuz wrote:
>
> On Wed, 2 Apr 2025 at 18:54, Melanie Plageman
> wrote:
> >
> > On Wed, Apr 2, 2025 at 6:26 AM Nazir Bilal Yavuz wrote:
> > >
> I don't have an example code right now. But what I mean is we may call
> ReadBufferExtended() in a loop for
On Fri, Apr 4, 2025 at 4:17 AM Nazir Bilal Yavuz wrote:
>
> Same on my end, v14 LGTM.
Cool. Pushed and marked as such in the CF app.
- Melanie
> On 3 Apr 2025, at 21:25, Melanie Plageman wrote:
>
> On Thu, Apr 3, 2025 at 11:17 AM Heikki Linnakangas wrote:
>>
>> I had a quick look at this. Looks good overall
Same here, this seemed like a good piece to bite into with my limited AIO
knowledge to learn more, and reading it over it seems
On Mon, Mar 31, 2025 at 3:27 PM Melanie Plageman
wrote:
>
> I think there might be another problem with the code. We only set
> cur_database in the loop in autoprewarm_databas_main() when it is 0
>
> if (cur_database != blk->database)
> {
> if (cur_database == 0)
>
Hi,
On Tue, 1 Apr 2025 at 05:14, Melanie Plageman wrote:
>
> On Mon, Mar 31, 2025 at 3:45 PM Melanie Plageman
> wrote:
> >
> > Whoops, this isn't right. It does work. I'm going to draft a version
> > suggesting slightly different variable naming and a couple comments to
> > make this more clear.
Hi,
On Fri, 4 Apr 2025 at 10:59, Daniel Gustafsson wrote:
>
> This patch is already marked Ready for Committer and I concur with that.
Same on my end, v14 LGTM.
--
Regards,
Nazir Bilal Yavuz
Microsoft
> On 3 Apr 2025, at 22:54, Melanie Plageman wrote:
> On Thu, Apr 3, 2025 at 4:22 PM Daniel Gustafsson wrote:
>> + while (p->pos < apw_state->prewarm_stop_idx)
>> + {
>> + BlockInfoRecord blk = p->block_info[p->pos];
>> +
>> + CHECK_FOR_INTERRUPTS();
>> Isn't checking inside this
On Thu, Apr 3, 2025 at 11:17 AM Heikki Linnakangas wrote:
>
> I had a quick look at this. Looks good overall, some small remarks:
Thanks for taking a look!
> v12-0001-Autoprewarm-global-objects-separately.patch
>
> > Instead, modify apw_load_buffers() to prewarm the shared objects in one
> > inv
On 03/04/2025 17:31, Melanie Plageman wrote:
Attached v12 fixes a bug Bilal found off-list in 0002 related to
handling invalid blocks.
I had a quick look at this. Looks good overall, some small remarks:
v12-0001-Autoprewarm-global-objects-separately.patch
Instead, modify apw_load_buffers() t
On Wed, Apr 2, 2025 at 8:25 PM Melanie Plageman
wrote:
>
> On Wed, Apr 2, 2025 at 3:34 PM Melanie Plageman
> wrote:
> > attached v10
>
> Attached v11 has an assortment of cosmetic updates.
Attached v12 fixes a bug Bilal found off-list in 0002 related to
handling invalid blocks.
- Melanie
From 7
On Wed, Apr 2, 2025 at 3:34 PM Melanie Plageman
wrote:
> attached v10
Attached v11 has an assortment of cosmetic updates.
- Melanie
From 3b1657a0ba51299fa0384a0032759f3661a2baca Mon Sep 17 00:00:00 2001
From: Melanie Plageman
Date: Mon, 31 Mar 2025 22:02:25 -0400
Subject: [PATCH v11 2/3] Refact
On Wed, Apr 2, 2025 at 6:26 AM Nazir Bilal Yavuz wrote:
>
> On Wed, 2 Apr 2025 at 01:36, Melanie Plageman
> wrote:
> >
> > I know you mentioned off-list that you don't like the handling of
> > global objects in my version, but I prefer doing it this way (even
> > though we have to check for in t
Hi,
On Wed, 2 Apr 2025 at 01:36, Melanie Plageman wrote:
>
> On Tue, Apr 1, 2025 at 8:50 AM Nazir Bilal Yavuz wrote:
> >
> > I am attaching v8, which is an updated version of the v7. I tried to
> > get rid of these local variables and refactored code to make logic
> > more straightforward instea
Hi,
On Wed, 2 Apr 2025 at 18:54, Melanie Plageman wrote:
>
> On Wed, Apr 2, 2025 at 6:26 AM Nazir Bilal Yavuz wrote:
> >
> > On Wed, 2 Apr 2025 at 01:36, Melanie Plageman
> > wrote:
> > >
> > > I know you mentioned off-list that you don't like the handling of
> > > global objects in my version
On Tue, Apr 1, 2025 at 8:50 AM Nazir Bilal Yavuz wrote:
>
> I am attaching v8, which is an updated version of the v7. I tried to
> get rid of these local variables and refactored code to make logic
> more straightforward instead of going back and forth.
>
> 0001 and 0002 are v8. 0003 is another re
Hi,
On Tue, 1 Apr 2025 at 05:14, Melanie Plageman wrote:
>
> On Mon, Mar 31, 2025 at 3:45 PM Melanie Plageman
> wrote:
> >
> > Whoops, this isn't right. It does work. I'm going to draft a version
> > suggesting slightly different variable naming and a couple comments to
> > make this more clear.
Hi,
Thank you for looking into this!
On Mon, 31 Mar 2025 at 17:42, Melanie Plageman
wrote:
>
> On Sun, Mar 30, 2025 at 10:01 AM Nazir Bilal Yavuz wrote:
> >
> > > Some review feedback on your v4: I don't think we need the
> > > rs_have_free_buffer per_buffer_data. We can just check
> > > have_f
Hi,
On Mon, 31 Mar 2025 at 21:15, Melanie Plageman
wrote:
>
> On Mon, Mar 31, 2025 at 12:27 PM Nazir Bilal Yavuz wrote:
> >
> > I worked on an alternative approach, I refactored code a bit. It does
> > not traverse the list two times and I think the code is more suitable
> > to use read streams
On Sat, Mar 29, 2025 at 4:44 PM Andres Freund wrote:
>
> How about having an iterator function operating on a pointer to iterator state
> that's used both by the main loop and the read stream callback? If the
> iterator reaches the next relation, it returns InvalidBlockNumber and the main
> loop s
On Fri, Nov 29, 2024 at 6:20 AM Nazir Bilal Yavuz wrote:
>
> v4 is attached.
I've started looking at this version. What I don't like about this
structure is that we are forced to increment the position in the array
of BlockInfoRecords in both the callback and the main loop in
autoprewarm_database
Hi,
On 2025-03-29 16:09:56 -0400, Melanie Plageman wrote:
> I've started looking at this version. What I don't like about this
> structure is that we are forced to increment the position in the array
> of BlockInfoRecords in both the callback and the main loop in
> autoprewarm_database_main(). The
On Sat, Mar 29, 2025 at 4:09 PM Melanie Plageman
wrote:
>
> One alternative is to loop through the array of BlockInfoRecords and
> get the start and end positions of the blocks in the arary for a
> single relation/fork combo. Then we could make the read stream and
> pass those two positions and th
On Fri, 29 Nov 2024 at 16:19, Nazir Bilal Yavuz wrote:
> v4 is attached.
Hi!
I feel like we are ready to mark this as RFC, WDYT?
--
Best regards,
Kirill Reshke
Hi,
On Mon, 2 Dec 2024 at 16:30, Andrey M. Borodin wrote:
>
> > On 2 Dec 2024, at 16:16, Kirill Reshke wrote:
> >
> > I feel like we are ready to mark this as RFC, WDYT?
>
> +1
Done.
--
Regards,
Nazir Bilal Yavuz
Microsoft
> On 2 Dec 2024, at 16:16, Kirill Reshke wrote:
>
> I feel like we are ready to mark this as RFC, WDYT?
+1
Best regards, Andrey Borodin.
Hi,
Thank you for looking into this.
On Fri, 29 Nov 2024 at 06:55, Kirill Reshke wrote:
> > + old_blk = &(p->block_info[p->pos - 1]);
> > + cur_blk = &(p->block_info[p->pos]);
> Should we Assert(p->pos > 0 && p->pos < *something*)
I think it is worth adding:
+ Assert(p->pos > 0 && p->pos < p-
On Wed, 27 Nov 2024 at 19:20, Nazir Bilal Yavuz wrote:
>
> Hi,
>
> Thank you for looking into this!
>
> On Wed, 27 Nov 2024 at 16:50, Matheus Alcantara wrote:
> > I've executed the same test of 5 databases with each of them having 1 table
> > of
> > 3GB of size and I've got very similar results.
On Wednesday, November 27th, 2024 at 11:19 AM, Nazir Bilal Yavuz
wrote:
> > v2-0001-Use-read-stream-in-autoprewarm.patch
> > + bool *rs_have_free_buffer = per_buffer_data;
> > +
> > +
> > + *rs_have_free_buffer = true;
> > +
> >
> > Not sure if I understand why this variable is needed, it seems
Hi,
Newer reviewer here, trying to understand more about the read stream API.
On Tuesday, November 26th, 2024 at 11:07 AM, Nazir Bilal Yavuz
wrote:
> Any feedback would be appreciated.
I've executed the same test of 5 databases with each of them having 1 table of
3GB of size and I've got very
Hi,
Thank you for looking into this!
On Wed, 27 Nov 2024 at 16:50, Matheus Alcantara wrote:
> I've executed the same test of 5 databases with each of them having 1 table of
> 3GB of size and I've got very similar results.
>
> I've also tested using a single database with 4 tables with ~60GB of s
Hi,
On Fri, 1 Nov 2024 at 21:06, Andrey M. Borodin wrote:
>
> > On 1 Nov 2024, at 12:51, Nazir Bilal Yavuz wrote:
> >
> > am not
> > sure whether 'BufferStrategyControl.lastFreeBuffer -
> > BufferStrategyControl.firstFreeBuffer' is safe to use.
>
> Ugh... it will work. But it seems to me too di
> On 1 Nov 2024, at 12:51, Nazir Bilal Yavuz wrote:
>
> am not
> sure whether 'BufferStrategyControl.lastFreeBuffer -
> BufferStrategyControl.firstFreeBuffer' is safe to use.
Ugh... it will work. But it seems to me too dirty hack. There's no scalable way
to know size of a free list.
Let's j
Hi,
Thanks for looking into this!
On Thu, 31 Oct 2024 at 22:02, Stepan Neretin wrote:
>
> At first A quick look it looks good. I will take a closer look at it
> tomorrow. Could you please let me know about the performance tests and
> graphics?
Sorry but I didn't understand what you mean by pe
Hi,
Thanks for looking into this!
On Thu, 31 Oct 2024 at 21:18, Andrey M. Borodin wrote:
>
> > On 8 Aug 2024, at 11:32, Nazir Bilal Yavuz wrote:
> >
> > Any feedback would be appreciated.
>
> I've took a look into the patch. It seems to me that you add new block
> numbers to the read stream un
Dear Nazir,
At first A quick look it looks good. I will take a closer look at it
tomorrow. Could you please let me know about the performance tests and
graphics?
Best regards, Stepan Neretin!
> On 8 Aug 2024, at 11:32, Nazir Bilal Yavuz wrote:
>
> Any feedback would be appreciated.
I've took a look into the patch. It seems to me that you add new block numbers
to the read stream until you have buffers. So when there are no more buffers
you will still have some queued blocks.
Mayb
43 matches
Mail list logo
