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
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.
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
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
> > >
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 =
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,
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
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
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
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
> >>>
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
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
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
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
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
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
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.
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.
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
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
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
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
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
P_TXN_COUNT_THRESHOLD MaxConnections
> >
> > Isn't using max-heap equally effective in finding the largest
> > transaction whether there are top-level or top-level plus
> > subtransactions? This comment indicates it is only effective when
> > there are subtransactions
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
>
> Isn't using max-heap equally effective in finding the largest
> transaction whether there are top-level or top-level plus
> subtransactions? This comment indicates it is only effective when
> there are subtransactions.
You're right. Updated the comment.
I've attached the updated patches.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
v11-0002-Add-functions-to-binaryheap-for-efficient-key-re.patch
Description: Binary data
v11-0001-Make-binaryheap-enlargeable.patch
Description: Binary data
v11-0003-Improve-eviction-algorithm-in-Reorderbuffer-usin.patch
Description: Binary data
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,
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
inaryheap-enlargeable.patch
Description: Binary data
v10-0003-Improve-eviction-algorithm-in-Reorderbuffer-usin.patch
Description: Binary data
v10-0002-Add-functions-to-binaryheap-for-efficient-key-re.patch
Description: Binary data
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
he enum and changed the logic.
>
> ~~~
>
> 12. struct ReorderBuffer
>
> + /* Max-heap for sizes of all top-level and sub transactions */
> + ReorderBufferMemTrackState memtrack_state;
> + binaryheap *txn_heap;
> +
>
> 12a.
> Why is this being referred to in the commit message and code comments
> as "max-heap" when the field is not called by that same name? Won't it
> be better to give the field a better name -- e.g. "txn_maxheap" or
> similar?
Not sure it helps increase readability. Other codes where we use
binaryheap use neither max nor min in the field name.
>
> ~
>
> 12b.
> This comment should also say that the heap is ordered by tx size --
> (e.g. the comparator is ReorderBufferTXNSizeCompare)
It seems to me the comment "/* Max-heap for sizes of all top-level and
sub transactions */" already mentions that, no? I'm not sure we need
to refer to the actual function name here.
I've attached new version patches.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
v9-0001-Make-binaryheap-enlargeable.patch
Description: Binary data
v9-0002-Add-functions-to-binaryheap-for-efficient-key-rem.patch
Description: Binary data
v9-0003-Improve-eviction-algorithm-in-Reorderbuffer-using.patch
Description: Binary data
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
lts on my environment (with
10M records and debug_logical_replication_streaming = 'immediate'):
HEAD:
68937.887 ms
69450.174 ms
68808.248 ms
v7 patch:
71280.783 ms
71673.101 ms
71330.898 ms
v8 patch:
68918.259 ms
68822.330 ms
68972.452 ms
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
v8-0001-Make-binaryheap-enlargeable.patch
Description: Binary data
v8-0002-Add-functions-to-binaryheap-for-efficient-key-rem.patch
Description: Binary data
v8-0003-Improve-eviction-algorithm-in-Reorderbuffer-using.patch
Description: Binary data
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
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
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
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
log that as a WARNING, similarly
> >> to checkpoints - there we only log "checkpoints too frequent, tune WAL
> >> limits", but perhaps we might do more here? Or maybe we could add the
> >> watermark to the system catalog?
> >
> > Interesting ideas.
&g
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,
>>
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
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,
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
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
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
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
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
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;
> > +
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
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
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
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;
+
+
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
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
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]
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
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
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
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
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
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
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
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
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
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
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
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
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 this might be quite slow, because we'll have
* to walk through all of them.
93 matches
Mail list logo
