On Thu, Apr 11, 2024 at 10:46 AM Hayato Kuroda (Fujitsu)
wrote:
>
> Dear Heikki,
>
> I also prototyped the idea, which has almost the same shape.
> I attached just in case, but we may not have to see.
>
> Few comments based on the experiment.
Thank you for reviewing the patch. I didn't include th
On 11/04/2024 11:20, Masahiko Sawada wrote:
On Thu, Apr 11, 2024 at 11:52 AM Masahiko Sawada wrote:
We can see 2% ~ 3% performance regressions compared to the current
HEAD, but it's much smaller than I expected. Given that we can make
the code simple, I think we can go with this direction.
P
On Thu, Apr 11, 2024 at 11:52 AM Masahiko Sawada wrote:
>
> We can see 2% ~ 3% performance regressions compared to the current
> HEAD, but it's much smaller than I expected. Given that we can make
> the code simple, I think we can go with this direction.
Pushed the patch and reverted binaryheap c
On Thu, Apr 11, 2024 at 10:32 AM Masahiko Sawada wrote:
>
> Hi,
>
> Sorry for the late reply, I took two days off.
>
> On Thu, Apr 11, 2024 at 6:20 AM Heikki Linnakangas wrote:
> >
> > On 10/04/2024 08:31, Amit Kapila wrote:
> > > On Wed, Apr 10, 2024 at 11:00 AM Heikki Linnakangas
> > > wrote:
Dear Heikki,
I also prototyped the idea, which has almost the same shape.
I attached just in case, but we may not have to see.
Few comments based on the experiment.
```
+ /* txn_heap is ordered by transaction size */
+ buffer->txn_heap = pairingheap_allocate(ReorderBufferTXNSizeCompa
Hi,
Sorry for the late reply, I took two days off.
On Thu, Apr 11, 2024 at 6:20 AM Heikki Linnakangas wrote:
>
> On 10/04/2024 08:31, Amit Kapila wrote:
> > On Wed, Apr 10, 2024 at 11:00 AM Heikki Linnakangas wrote:
> >>
> >> On 10/04/2024 07:45, Michael Paquier wrote:
> >>> On Tue, Apr 09, 202
On 11/04/2024 01:37, Michael Paquier wrote:
On Thu, Apr 11, 2024 at 12:20:55AM +0300, Heikki Linnakangas wrote:
To move this forward, here's a patch to switch to a pairing heap. In my very
quick testing, with the performance test cases posted earlier in this thread
[1] [2], I'm seeing no meaning
On Thu, Apr 11, 2024 at 12:20:55AM +0300, Heikki Linnakangas wrote:
> To move this forward, here's a patch to switch to a pairing heap. In my very
> quick testing, with the performance test cases posted earlier in this thread
> [1] [2], I'm seeing no meaningful performance difference between this a
On 10/04/2024 08:31, Amit Kapila wrote:
On Wed, Apr 10, 2024 at 11:00 AM Heikki Linnakangas wrote:
On 10/04/2024 07:45, Michael Paquier wrote:
On Tue, Apr 09, 2024 at 09:16:53PM -0700, Jeff Davis wrote:
On Wed, 2024-04-10 at 12:13 +0900, Michael Paquier wrote:
Wouldn't the best way forward
On Wed, 2024-04-10 at 08:30 +0300, Heikki Linnakangas wrote:
> My #1 choice would be to write a patch to switch the pairing heap,
> performance test that, and revert the binary heap changes.
Sounds good to me. I would expect it to perform better than the extra
hash table, if anything.
It also ha
On Wed, Apr 10, 2024 at 11:00 AM Heikki Linnakangas wrote:
>
> On 10/04/2024 07:45, Michael Paquier wrote:
> > On Tue, Apr 09, 2024 at 09:16:53PM -0700, Jeff Davis wrote:
> >> On Wed, 2024-04-10 at 12:13 +0900, Michael Paquier wrote:
> >>> Wouldn't the best way forward be to revert
> >>> 5bec1d6bc
On 10/04/2024 07:45, Michael Paquier wrote:
On Tue, Apr 09, 2024 at 09:16:53PM -0700, Jeff Davis wrote:
On Wed, 2024-04-10 at 12:13 +0900, Michael Paquier wrote:
Wouldn't the best way forward be to revert
5bec1d6bc5e3 and revisit the whole in v18?
Also consider commits b840508644 and bcb14f4a
On Tue, Apr 09, 2024 at 09:16:53PM -0700, Jeff Davis wrote:
> On Wed, 2024-04-10 at 12:13 +0900, Michael Paquier wrote:
>> Wouldn't the best way forward be to revert
>> 5bec1d6bc5e3 and revisit the whole in v18?
>
> Also consider commits b840508644 and bcb14f4abc.
Indeed. These are also linked.
On Wed, 2024-04-10 at 12:13 +0900, Michael Paquier wrote:
> Wouldn't the best way forward be to revert
> 5bec1d6bc5e3 and revisit the whole in v18?
That's a reasonable conclusion. Also consider commits b840508644 and
bcb14f4abc.
I had tried to come up with a narrower fix, and I think it's alread
On Tue, Apr 09, 2024 at 06:24:43PM -0700, Jeff Davis wrote:
> I had suggested that the heap could track the element indexes for
> efficient update/removal, but that would be a change to the
> binaryheap.h API, which would require some discussion (and possibly not
> be acceptable post-freeze).
>
>
On Tue, 2024-04-09 at 23:49 +0300, Heikki Linnakangas wrote:
>
> I wonder why this doesn't use the existing pairing heap
> implementation? I would assume that to be at least as good as the
> binary
> heap + hash table
I agree that an additional hash table is not needed -- there's already
a hash
On 09/04/2024 21:04, Jeff Davis wrote:
On Fri, 2024-04-05 at 16:58 +0900, Masahiko Sawada wrote:
I have some further comments and I believe changes are required for
v17.
I also noticed that the simplehash is declared in binaryheap.h with
"SH_SCOPE extern", which seems wrong. Attached is a roug
On Fri, 2024-04-05 at 16:58 +0900, Masahiko Sawada wrote:
> > I have some further comments and I believe changes are required for
> > v17.
I also noticed that the simplehash is declared in binaryheap.h with
"SH_SCOPE extern", which seems wrong. Attached is a rough patch to
bring the declarations i
On Mon, 2024-04-08 at 21:29 +0900, Masahiko Sawada wrote:
> For v17, changes for #2 are smaller, but I'm concerned
> that the new API that requires a hash function to be able to use
> binaryheap_update_{up|down} might not be user friendly.
The only API change in 02 is accepting a hash callback rat
On Sat, Apr 6, 2024 at 5:44 AM Jeff Davis wrote:
>
> >
> > It sounds like a data structure that mixes the hash table and the
> > binary heap and we use it as the main storage (e.g. for
> > ReorderBufferTXN) instead of using the binary heap as the secondary
> > data structure. IIUC with your idea,
On Fri, 2024-04-05 at 16:58 +0900, Masahiko Sawada wrote:
> IIUC for example in ReorderBuffer, the heap key is transaction size
> and the hash key is xid.
Yes.
>
> I see your point. It assumes that the bh_node_type is a pointer or at
> least unique. So it cannot work with Datum being a value.
R
On Fri, Apr 5, 2024 at 2:55 AM Jeff Davis wrote:
>
> On Thu, 2024-04-04 at 17:28 +0900, Masahiko Sawada wrote:
> > > Perhaps it's not worth the effort though, if performance is already
> > > good enough?
> >
> > Yeah, it would be better to measure the overhead first. I'll do that.
>
> I have some
On Thu, 2024-04-04 at 10:55 -0700, Jeff Davis wrote:
> * Make a proper indexed binary heap API that accepts a hash
> function
> and provides both heap methods and HT methods that operate based on
> values (transaction size and transaction ID, respectively).
> * Get rid of ReorderBuffer->by_txn
On Thu, 2024-04-04 at 17:28 +0900, Masahiko Sawada wrote:
> > Perhaps it's not worth the effort though, if performance is already
> > good enough?
>
> Yeah, it would be better to measure the overhead first. I'll do that.
I have some further comments and I believe changes are required for
v17.
An
On Thu, Apr 4, 2024 at 1:54 PM Jeff Davis wrote:
>
> On Thu, 2024-04-04 at 09:31 +0900, Masahiko Sawada wrote:
> > IIUC, with your suggestion, sift_{up|down} needs to update the
> > heap_index field as well. Does it mean that the caller needs to pass
> > the address of heap_index down to sift_{up|
On Thu, 2024-04-04 at 09:31 +0900, Masahiko Sawada wrote:
> IIUC, with your suggestion, sift_{up|down} needs to update the
> heap_index field as well. Does it mean that the caller needs to pass
> the address of heap_index down to sift_{up|down}?
I'm not sure quite how binaryheap should be changed.
Hi,
On Thu, Apr 4, 2024 at 2:32 AM Jeff Davis wrote:
>
> On Wed, 2024-04-03 at 01:45 -0700, Jeff Davis wrote:
> > I suggest that you add a "heap_index" field to ReorderBufferTXN that
> > would point to the index into the heap's array (the same as
> > bh_nodeidx_entry.index in your patch). Each ti
On Wed, 2024-04-03 at 01:45 -0700, Jeff Davis wrote:
> I suggest that you add a "heap_index" field to ReorderBufferTXN that
> would point to the index into the heap's array (the same as
> bh_nodeidx_entry.index in your patch). Each time an element moves
> within the heap array, just follow the poin
On Mon, 2024-04-01 at 12:42 +0900, Masahiko Sawada wrote:
> While reviewing the patches, I realized the comment of
> binearyheap_allocate() should also be updated. So I've attached the
> new patches.
In sift_{up|down}, each loop iteration calls set_node(), and each call
to set_node does a hash loo
On Mon, Apr 1, 2024 at 11:26 AM Masahiko Sawada wrote:
>
> On Fri, Mar 29, 2024 at 7:37 PM Amit Kapila wrote:
> >
> > On Fri, Mar 29, 2024 at 12:13 PM Masahiko Sawada
> > wrote:
> > >
> > > On Fri, Mar 29, 2024 at 2:09 PM vignesh C wrote:
> > > >
> > > > On Tue, 26 Mar 2024 at 10:05, Masahiko
On Fri, Mar 29, 2024 at 8:48 PM Hayato Kuroda (Fujitsu)
wrote:
>
> Dear Sawada-san,
>
> > Agreed.
> >
> > I think the patch is in good shape. I'll push the patch with the
> > suggestion next week, barring any objections.
>
> Thanks for working on this. Agreed it is committable.
> Few minor comment
On Fri, Mar 29, 2024 at 7:37 PM Amit Kapila wrote:
>
> On Fri, Mar 29, 2024 at 12:13 PM Masahiko Sawada
> wrote:
> >
> > On Fri, Mar 29, 2024 at 2:09 PM vignesh C wrote:
> > >
> > > On Tue, 26 Mar 2024 at 10:05, Masahiko Sawada
> > > wrote:
> > > >
> > > > On Thu, Mar 14, 2024 at 12:02 PM Mas
Dear Sawada-san,
> Agreed.
>
> I think the patch is in good shape. I'll push the patch with the
> suggestion next week, barring any objections.
Thanks for working on this. Agreed it is committable.
Few minor comments:
```
+ * Either txn or change must be non-NULL at least. We update the memory
On Fri, Mar 29, 2024 at 12:13 PM Masahiko Sawada wrote:
>
> On Fri, Mar 29, 2024 at 2:09 PM vignesh C wrote:
> >
> > On Tue, 26 Mar 2024 at 10:05, Masahiko Sawada wrote:
> > >
> > > On Thu, Mar 14, 2024 at 12:02 PM Masahiko Sawada
> > > wrote:
> > > >
> > > >
> > > > I've attached new version
On Fri, Mar 29, 2024 at 2:09 PM vignesh C wrote:
>
> On Tue, 26 Mar 2024 at 10:05, Masahiko Sawada wrote:
> >
> > On Thu, Mar 14, 2024 at 12:02 PM Masahiko Sawada
> > wrote:
> > >
> > >
> > > I've attached new version patches.
> >
> > Since the previous patch conflicts with the current HEAD, I'
On Tue, 26 Mar 2024 at 10:05, Masahiko Sawada wrote:
>
> On Thu, Mar 14, 2024 at 12:02 PM Masahiko Sawada
> wrote:
> >
> >
> > I've attached new version patches.
>
> Since the previous patch conflicts with the current HEAD, I've
> attached the rebased patches.
Thanks for the updated patch.
One
On Tue, Mar 5, 2024 at 8:50 AM vignesh C wrote:
>
> On Wed, 28 Feb 2024 at 11:40, Amit Kapila wrote:
> >
> > On Mon, Feb 26, 2024 at 7:54 PM Masahiko Sawada
> > wrote:
> > >
> >
> > A few comments on 0003:
> > ===
> > 1.
> > +/*
> > + * Threshold of the total number of top-level
On Thu, Mar 14, 2024 at 12:02 PM Masahiko Sawada wrote:
>
>
> I've attached new version patches.
Since the previous patch conflicts with the current HEAD, I've
attached the rebased patches.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
v10-0001-Make-binaryheap-enlar
On Wed, Mar 13, 2024 at 11:23 AM Peter Smith wrote:
>
> On Wed, Mar 13, 2024 at 12:48 PM Masahiko Sawada
> wrote:
> >
> > On Wed, Mar 13, 2024 at 10:15 AM Peter Smith wrote:
> > >
> > > On Tue, Mar 12, 2024 at 4:23 PM Masahiko Sawada
> > > wrote:
> > > >
> > > > On Fri, Mar 8, 2024 at 12:58 P
On Mon, Mar 11, 2024 at 3:04 PM Peter Smith wrote:
>
> Here are some review comments for v8-0003
>
> ==
> 0. GENERAL -- why the state enum?
>
> This patch introduced a new ReorderBufferMemTrackState with 2 states
> (REORDER_BUFFER_MEM_TRACK_NO_MAXHEAP,
> REORDER_BUFFER_MEM_TRACK_MAINTAIN_MAXHE
On Wed, Mar 13, 2024 at 12:48 PM Masahiko Sawada wrote:
>
> On Wed, Mar 13, 2024 at 10:15 AM Peter Smith wrote:
> >
> > On Tue, Mar 12, 2024 at 4:23 PM Masahiko Sawada
> > wrote:
> > >
> > > On Fri, Mar 8, 2024 at 12:58 PM Peter Smith wrote:
> > > >
> > ...
> > > > > > 5.
> > > > > > + *
> > >
On Wed, Mar 13, 2024 at 10:15 AM Peter Smith wrote:
>
> On Tue, Mar 12, 2024 at 4:23 PM Masahiko Sawada wrote:
> >
> > On Fri, Mar 8, 2024 at 12:58 PM Peter Smith wrote:
> > >
> ...
> > > > > 5.
> > > > > + *
> > > > > + * If 'indexed' is true, we create a hash table to track of each
> > > > >
On Tue, Mar 12, 2024 at 4:23 PM Masahiko Sawada wrote:
>
> On Fri, Mar 8, 2024 at 12:58 PM Peter Smith wrote:
> >
...
> > > > 5.
> > > > + *
> > > > + * If 'indexed' is true, we create a hash table to track of each node's
> > > > + * index in the heap, enabling to perform some operations such as
On Fri, Mar 8, 2024 at 12:58 PM Peter Smith wrote:
>
> On Thu, Mar 7, 2024 at 2:16 PM Masahiko Sawada wrote:
> >
> > On Tue, Mar 5, 2024 at 3:28 PM Peter Smith wrote:
> > >
>
> > > 4a.
> > > The comment in simplehash.h says
> > > * The following parameters are only relevant when SH_DEFINE is
Here are some review comments for v8-0003
==
0. GENERAL -- why the state enum?
This patch introduced a new ReorderBufferMemTrackState with 2 states
(REORDER_BUFFER_MEM_TRACK_NO_MAXHEAP,
REORDER_BUFFER_MEM_TRACK_MAINTAIN_MAXHEAP)
It's the same as having a boolean flag OFF/ON, so I didn't see
On Thu, Mar 7, 2024 at 2:16 PM Masahiko Sawada wrote:
>
> On Tue, Mar 5, 2024 at 3:28 PM Peter Smith wrote:
> >
> > 4a.
> > The comment in simplehash.h says
> > * The following parameters are only relevant when SH_DEFINE is defined:
> > * - SH_KEY - ...
> > * - SH_EQUAL(table, a, b) - .
On Tue, Mar 5, 2024 at 3:28 PM Peter Smith wrote:
>
> Hi, here are some review comments for v7-0002
>
> ==
> Commit Message
>
> 1.
> This commit adds a hash table to binaryheap in order to track of
> positions of each nodes in the binaryheap. That way, by using newly
> added functions such as
On Tue, Mar 5, 2024 at 12:20 PM vignesh C wrote:
>
> On Wed, 28 Feb 2024 at 11:40, Amit Kapila wrote:
> >
> > On Mon, Feb 26, 2024 at 7:54 PM Masahiko Sawada
> > wrote:
> > >
> >
> > A few comments on 0003:
> > ===
> > 1.
> > +/*
> > + * Threshold of the total number of top-leve
On Tue, Mar 5, 2024 at 11:25 AM Peter Smith wrote:
>
> Hi, Here are some review comments for v7-0001
Thank you for reviewing the patch.
>
> 1.
> /*
> * binaryheap_free
> *
> * Releases memory used by the given binaryheap.
> */
> void
> binaryheap_free(binaryheap *heap)
> {
> pfree(heap);
> }
Hi, here are some review comments for v7-0002
==
Commit Message
1.
This commit adds a hash table to binaryheap in order to track of
positions of each nodes in the binaryheap. That way, by using newly
added functions such as binaryheap_update_up() etc., both updating a
key and removing a node
On Wed, 28 Feb 2024 at 11:40, Amit Kapila wrote:
>
> On Mon, Feb 26, 2024 at 7:54 PM Masahiko Sawada wrote:
> >
>
> A few comments on 0003:
> ===
> 1.
> +/*
> + * Threshold of the total number of top-level and sub transactions
> that controls
> + * whether we switch the memory tra
Hi, Here are some review comments for v7-0001
1.
/*
* binaryheap_free
*
* Releases memory used by the given binaryheap.
*/
void
binaryheap_free(binaryheap *heap)
{
pfree(heap);
}
Shouldn't the above function (not modified by the patch) also firstly
free the memory allocated for the heap->bh_
On Wed, Feb 28, 2024 at 3:10 PM Amit Kapila wrote:
>
> On Mon, Feb 26, 2024 at 7:54 PM Masahiko Sawada wrote:
> >
>
Thank you for the comments!
> A few comments on 0003:
> ===
> 1.
> +/*
> + * Threshold of the total number of top-level and sub transactions
> that controls
> + *
On Mon, Feb 26, 2024 at 7:54 PM Masahiko Sawada wrote:
>
A few comments on 0003:
===
1.
+/*
+ * Threshold of the total number of top-level and sub transactions
that controls
+ * whether we switch the memory track state. While the MAINTAIN_HEAP state is
+ * effective when there are
On Mon, 26 Feb 2024 at 12:33, Masahiko Sawada wrote:
>
> On Fri, Feb 23, 2024 at 6:24 PM vignesh C wrote:
> >
> > On Fri, 9 Feb 2024 at 20:51, Masahiko Sawada wrote:
> > >
> > >
> > > I think this performance regression is not acceptable. In this
> > > workload, one transaction has 10k subtransa
On Mon, Feb 26, 2024 at 6:43 PM Tomas Vondra
wrote:
>
>
>
> On 2/26/24 07:46, Masahiko Sawada wrote:
> > On Sat, Feb 24, 2024 at 1:29 AM Tomas Vondra
> > wrote:
> >>...
> >>
> >> overall design
> >> --
> >>
> >> As for the design, I agree with the approach of using a binaryheap to
> >
On 2/26/24 07:46, Masahiko Sawada wrote:
> On Sat, Feb 24, 2024 at 1:29 AM Tomas Vondra
> wrote:
>>...
>>
>> overall design
>> --
>>
>> As for the design, I agree with the approach of using a binaryheap to
>> track transactions by size. When going over the thread history,
>> describ
On Fri, Feb 23, 2024 at 6:24 PM vignesh C wrote:
>
> On Fri, 9 Feb 2024 at 20:51, Masahiko Sawada wrote:
> >
> >
> > I think this performance regression is not acceptable. In this
> > workload, one transaction has 10k subtransactions and the logical
> > decoding becomes quite slow if logical_deco
On Sat, Feb 24, 2024 at 1:29 AM Tomas Vondra
wrote:
>
> Hi,
>
> I did a basic review and testing of this patch today. Overall I think
> the patch is in very good shape - I agree with the tradeoffs it makes,
> and I like the approach in general. I do have a couple minor comments
> about the code, a
Hi,
I did a basic review and testing of this patch today. Overall I think
the patch is in very good shape - I agree with the tradeoffs it makes,
and I like the approach in general. I do have a couple minor comments
about the code, and then maybe a couple thoughts about the approach.
First, some
On Fri, 9 Feb 2024 at 20:51, Masahiko Sawada wrote:
>
> On Thu, Feb 8, 2024 at 6:33 PM Hayato Kuroda (Fujitsu)
> wrote:
> >
> > Dear Sawada-san,
> >
> > Thanks for making v3 patchset. I have also benchmarked the case [1].
> > Below results are the average of 5th, there are almost the same result
On Sat, Feb 10, 2024 at 2:23 AM Masahiko Sawada
wrote:
> On Fri, Feb 9, 2024 at 7:35 PM Ajin Cherian wrote:
> >
> >
> >
> > On Tue, Feb 6, 2024 at 5:06 PM Masahiko Sawada
> wrote:
> >>
> >>
> >> I've attached the new version patch set.
> >>
> >> Regards,
> >>
> >>
> >> --
> >> Masahiko Sawada
>
On Fri, Feb 9, 2024 at 7:35 PM Ajin Cherian wrote:
>
>
>
> On Tue, Feb 6, 2024 at 5:06 PM Masahiko Sawada wrote:
>>
>>
>> I've attached the new version patch set.
>>
>> Regards,
>>
>>
>> --
>> Masahiko Sawada
>> Amazon Web Services: https://aws.amazon.com
>
>
> Thanks for the patch. I reviewed th
On Thu, Feb 8, 2024 at 6:33 PM Hayato Kuroda (Fujitsu)
wrote:
>
> Dear Sawada-san,
>
> Thanks for making v3 patchset. I have also benchmarked the case [1].
> Below results are the average of 5th, there are almost the same result
> even when median is used for the comparison. On my env, the regress
On Tue, Feb 6, 2024 at 5:06 PM Masahiko Sawada
wrote:
>
> I've attached the new version patch set.
>
> Regards,
>
>
> --
> Masahiko Sawada
> Amazon Web Services: https://aws.amazon.com
Thanks for the patch. I reviewed that patch and did minimal testing and it
seems to show the speed up as claim
Dear Sawada-san,
Thanks for making v3 patchset. I have also benchmarked the case [1].
Below results are the average of 5th, there are almost the same result
even when median is used for the comparison. On my env, the regression
cannot be seen.
HEAD (1e285a5) HEAD + v3 patches difference
10
On Fri, Feb 2, 2024 at 5:16 PM Masahiko Sawada wrote:
>
> On Fri, Feb 2, 2024 at 1:59 PM Shubham Khanna
> wrote:
> >
> > On Fri, Jan 26, 2024 at 2:07 PM Masahiko Sawada
> > wrote:
> > >
> > > On Wed, Dec 20, 2023 at 12:11 PM Amit Kapila
> > > wrote:
> > > >
> > > > On Wed, Dec 20, 2023 at 6:4
Dear Sawada-san,
> Thank you for the review comments!
>
> >
> > A comment for 0001:
> >
> > 01.
> > ```
> > +static void
> > +bh_enlarge_node_array(binaryheap *heap)
> > +{
> > +if (heap->bh_size < heap->bh_space)
> > +return;
> > +
> > +heap->bh_space *= 2;
> > +heap->bh_node
On Fri, Feb 2, 2024 at 1:59 PM Shubham Khanna
wrote:
>
> On Fri, Jan 26, 2024 at 2:07 PM Masahiko Sawada wrote:
> >
> > On Wed, Dec 20, 2023 at 12:11 PM Amit Kapila
> > wrote:
> > >
> > > On Wed, Dec 20, 2023 at 6:49 AM Masahiko Sawada
> > > wrote:
> > > >
> > > > On Tue, Dec 19, 2023 at 8:02
Hi,
On Wed, Jan 31, 2024 at 5:32 PM vignesh C wrote:
>
> On Tue, 30 Jan 2024 at 13:37, Masahiko Sawada wrote:
> >
> > On Fri, Jan 26, 2024 at 5:36 PM Masahiko Sawada
> > wrote:
> > >
> > > On Wed, Dec 20, 2023 at 12:11 PM Amit Kapila
> > > wrote:
> > > >
> > > > On Wed, Dec 20, 2023 at 6:49
On Wed, Jan 31, 2024 at 2:18 PM Hayato Kuroda (Fujitsu)
wrote:
>
> Dear Sawada-san,
>
> I have started to read your patches. Here are my initial comments.
> At least, all subscription tests were passed on my env.
Thank you for the review comments!
>
> A comment for 0001:
>
> 01.
> ```
> +static
On Fri, Jan 26, 2024 at 2:07 PM Masahiko Sawada wrote:
>
> On Wed, Dec 20, 2023 at 12:11 PM Amit Kapila wrote:
> >
> > On Wed, Dec 20, 2023 at 6:49 AM Masahiko Sawada
> > wrote:
> > >
> > > On Tue, Dec 19, 2023 at 8:02 PM Amit Kapila
> > > wrote:
> > > >
> > > > On Tue, Dec 19, 2023 at 8:31 A
On Tue, 30 Jan 2024 at 13:37, Masahiko Sawada wrote:
>
> On Fri, Jan 26, 2024 at 5:36 PM Masahiko Sawada wrote:
> >
> > On Wed, Dec 20, 2023 at 12:11 PM Amit Kapila
> > wrote:
> > >
> > > On Wed, Dec 20, 2023 at 6:49 AM Masahiko Sawada
> > > wrote:
> > > >
> > > > On Tue, Dec 19, 2023 at 8:02
Dear Sawada-san,
I have started to read your patches. Here are my initial comments.
At least, all subscription tests were passed on my env.
A comment for 0001:
01.
```
+static void
+bh_enlarge_node_array(binaryheap *heap)
+{
+if (heap->bh_size < heap->bh_space)
+return;
+
+heap->
On Fri, Jan 26, 2024 at 5:36 PM Masahiko Sawada wrote:
>
> On Wed, Dec 20, 2023 at 12:11 PM Amit Kapila wrote:
> >
> > On Wed, Dec 20, 2023 at 6:49 AM Masahiko Sawada
> > wrote:
> > >
> > > On Tue, Dec 19, 2023 at 8:02 PM Amit Kapila
> > > wrote:
> > > >
> > > > On Tue, Dec 19, 2023 at 8:31 A
On Wed, Dec 20, 2023 at 12:11 PM Amit Kapila wrote:
>
> On Wed, Dec 20, 2023 at 6:49 AM Masahiko Sawada wrote:
> >
> > On Tue, Dec 19, 2023 at 8:02 PM Amit Kapila wrote:
> > >
> > > On Tue, Dec 19, 2023 at 8:31 AM Masahiko Sawada
> > > wrote:
> > > >
> > > > On Sun, Dec 17, 2023 at 11:40 AM Am
2024-01 Commitfest.
Hi, This patch has a CF status of "Needs Review" [1], but it seems
like there was some CFbot test failures last time it was run [2].
Please have a look and post an updated version if necessary.
==
[1] https://commitfest.postgresql.org/46/4699/
[2] https://cirrus-ci.com/git
On Wed, Dec 20, 2023 at 6:49 AM Masahiko Sawada wrote:
>
> On Tue, Dec 19, 2023 at 8:02 PM Amit Kapila wrote:
> >
> > On Tue, Dec 19, 2023 at 8:31 AM Masahiko Sawada
> > wrote:
> > >
> > > On Sun, Dec 17, 2023 at 11:40 AM Amit Kapila
> > > wrote:
> > > >
> > > >
> > > > The individual transac
On Tue, Dec 19, 2023 at 8:02 PM Amit Kapila wrote:
>
> On Tue, Dec 19, 2023 at 8:31 AM Masahiko Sawada wrote:
> >
> > On Sun, Dec 17, 2023 at 11:40 AM Amit Kapila
> > wrote:
> > >
> > >
> > > The individual transactions shouldn't cross
> > > 'logical_decoding_work_mem'. I got a bit confused by
On Tue, Dec 19, 2023 at 8:31 AM Masahiko Sawada wrote:
>
> On Sun, Dec 17, 2023 at 11:40 AM Amit Kapila wrote:
> >
> >
> > The individual transactions shouldn't cross
> > 'logical_decoding_work_mem'. I got a bit confused by your proposal to
> > maintain the lists: "...splitting it into two lists:
On Sun, Dec 17, 2023 at 11:40 AM Amit Kapila wrote:
>
> On Fri, Dec 15, 2023 at 11:29 AM Masahiko Sawada
> wrote:
> >
> > On Fri, Dec 15, 2023 at 12:37 PM Amit Kapila
> > wrote:
> > >
> > > On Wed, Dec 13, 2023 at 6:01 AM Masahiko Sawada
> > > wrote:
> > > >
> > >
> > > IIUC, you are giving
On Fri, Dec 15, 2023 at 11:29 AM Masahiko Sawada wrote:
>
> On Fri, Dec 15, 2023 at 12:37 PM Amit Kapila wrote:
> >
> > On Wed, Dec 13, 2023 at 6:01 AM Masahiko Sawada
> > wrote:
> > >
> >
> > IIUC, you are giving preference to multiple list ideas as compared to
> > (a) because you don't need t
On Sat, Dec 16, 2023 at 1:36 AM Euler Taveira wrote:
>
> On Fri, Dec 15, 2023, at 9:11 AM, Masahiko Sawada wrote:
>
>
> I assume you mean to add ReorderBufferTXN entries to the binaryheap
> and then build it by comparing their sizes (i.e. txn->size). But
> ReorderBufferTXN entries are removed and
On Fri, Dec 15, 2023, at 9:11 AM, Masahiko Sawada wrote:
>
> I assume you mean to add ReorderBufferTXN entries to the binaryheap
> and then build it by comparing their sizes (i.e. txn->size). But
> ReorderBufferTXN entries are removed and deallocated once the
> transaction finished. How can we eff
On Fri, Dec 15, 2023 at 2:59 PM Masahiko Sawada wrote:
>
> On Fri, Dec 15, 2023 at 12:37 PM Amit Kapila wrote:
> >
> > On Wed, Dec 13, 2023 at 6:01 AM Masahiko Sawada
> > wrote:
> > >
> > > On Tue, Dec 12, 2023 at 1:33 PM Dilip Kumar wrote:
> > > >
> > > > On Tue, Dec 12, 2023 at 9:01 AM Masah
On Fri, Dec 15, 2023 at 7:10 PM Alvaro Herrera wrote:
>
> On 2023-Dec-12, Masahiko Sawada wrote:
>
> > To deal with this problem, I initially thought of the idea (a)
> > mentioned in the comment; use a binary heap to maintain the
> > transactions sorted by the amount of changes or the size. But it
On 2023-Dec-12, Masahiko Sawada wrote:
> To deal with this problem, I initially thought of the idea (a)
> mentioned in the comment; use a binary heap to maintain the
> transactions sorted by the amount of changes or the size. But it seems
> not a good idea to try maintaining all transactions by i
On Fri, Dec 15, 2023 at 12:37 PM Amit Kapila wrote:
>
> On Wed, Dec 13, 2023 at 6:01 AM Masahiko Sawada wrote:
> >
> > On Tue, Dec 12, 2023 at 1:33 PM Dilip Kumar wrote:
> > >
> > > On Tue, Dec 12, 2023 at 9:01 AM Masahiko Sawada
> > > wrote:
> > > >
> > > > I've heard the report internally th
On Wed, Dec 13, 2023 at 6:01 AM Masahiko Sawada wrote:
>
> On Tue, Dec 12, 2023 at 1:33 PM Dilip Kumar wrote:
> >
> > On Tue, Dec 12, 2023 at 9:01 AM Masahiko Sawada
> > wrote:
> > >
> > > I've heard the report internally that replication lag became huge when
> > > decoding transactions each co
On Wed, Dec 13, 2023 at 6:01 AM Masahiko Sawada wrote:
>
> > Thanks for working on this, I think it would be good to test other
> > scenarios as well where this might have some negative impact and see
> > where we stand.
>
> Agreed.
>
> > 1) A scenario where suppose you have one very large transac
On Tue, Dec 12, 2023 at 1:33 PM Dilip Kumar wrote:
>
> On Tue, Dec 12, 2023 at 9:01 AM Masahiko Sawada wrote:
> >
> > Hi all,
> >
> > As the comment of ReorderBufferLargestTXN() says, it's very slow with
> > many subtransactions:
> >
> > /*
> > * Find the largest transaction (toplevel or subxact
On Tue, Dec 12, 2023 at 9:01 AM Masahiko Sawada wrote:
>
> Hi all,
>
> As the comment of ReorderBufferLargestTXN() says, it's very slow with
> many subtransactions:
>
> /*
> * Find the largest transaction (toplevel or subxact) to evict (spill to
> disk).
> *
> * XXX With many subtransactions t
92 matches
Mail list logo
