On Fri, Apr 25, 2025 at 4:05 PM shveta malik wrote:
>
> >
> > Here is V30 patch set includes the following changes:
> >
>
> Thank You for the patch, please find few concerns:
>
Please find few more concerns:
3)
In get_candidate_xid(), we first set candidate_xid_time and later
candidate_xid. And
>
> Here is V30 patch set includes the following changes:
>
Thank You for the patch, please find few concerns:
1)
By looking at code of ApplyLauncherMain(), it appears that we stopped
advancing shared-slot's xmin if any of the subscriptions with
retain_conflict_info is disabled. If a subscription
On Thu, Apr 24, 2025 at 6:11 PM Zhijie Hou (Fujitsu)
wrote:
> > Few comments for patch004:
> > Config.sgml:
> > 1)
> > +
> > +Maximum duration (in milliseconds) for which conflict
> > +information can be retained for conflict detection by the apply
> > worker.
> > +
On Wed, Apr 16, 2025 at 10:30 AM shveta malik wrote:
>
> On Wed, Mar 26, 2025 at 4:17 PM Zhijie Hou (Fujitsu)
> wrote:
> >
> > Here's a rebased version of the patch series.
> >
>
Few comments for patch004:
Config.sgml:
1)
+
+Maximum duration (in milliseconds) for which conflict
+
On Wed, Mar 26, 2025 at 4:17 PM Zhijie Hou (Fujitsu)
wrote:
>
> Here's a rebased version of the patch series.
>
Thanks for the patches.
While testing the GUC "max_conflict_retention_duration", I noticed a
behavior that seems to bypass its intended purpose.
On Pub, if a txn is stuck in the COMMI
On Wed, Mar 26, 2025 at 4:17 PM Zhijie Hou (Fujitsu)
wrote:
>
> Here's a rebased version of the patch series.
>
Thanks Hou-San for the patches. I am going through this long thread
and patches. One doubt I have is whenever there is a chance of
conflict-slot update (either xmin or possibility of it
Dear Hou,
Thanks for updating the patch! I can finally come back to the thread.
Regarding the max_retain_conflict_duration, I prefer GUC approach because it has
already had a mechanism for converting unit: subscription option does not have
it.
Below part contains my comments:
01. check_new_clu
On Thu, 20 Feb 2025 at 12:50, Zhijie Hou (Fujitsu)
wrote:
>
>
> Here is the v28 patch set, which converts the subscription option
> max_conflict_retention_duration into a GUC. Other logic remains unchanged.
After discussing with Hou internally, I have moved this to the next
CommitFest since it wi
On Mon, Feb 10, 2025 at 2:45 PM Amit Kapila wrote:
>
> On Mon, Feb 10, 2025 at 10:26 AM Dilip Kumar wrote:
> >
> > On Fri, Feb 7, 2025 at 11:17 AM Amit Kapila wrote:
> >
> > >
> > > > I'm not really sure that these behaviors are the expected behavior of
> > > > users who set max_conflict_retenti
On Mon, Feb 10, 2025 at 10:26 AM Dilip Kumar wrote:
>
> On Fri, Feb 7, 2025 at 11:17 AM Amit Kapila wrote:
>
> >
> > > I'm not really sure that these behaviors are the expected behavior of
> > > users who set max_conflict_retention_duration to some subscriptions.
> > > Or I might have set the wro
On Fri, Feb 7, 2025 at 11:17 AM Amit Kapila wrote:
>
> On Fri, Feb 7, 2025 at 2:18 AM Masahiko Sawada wrote:
> >
> > I'd like to confirm what users would expect of this
> > max_conflict_retention_duration option and it works as expected. IIUC
> > users would want to use this option when they want
On Thu, Feb 6, 2025 at 9:47 PM Amit Kapila wrote:
>
> On Fri, Feb 7, 2025 at 2:18 AM Masahiko Sawada wrote:
> >
> > I'd like to confirm what users would expect of this
> > max_conflict_retention_duration option and it works as expected. IIUC
> > users would want to use this option when they want
On Fri, Feb 7, 2025 at 2:18 AM Masahiko Sawada wrote:
>
> I'd like to confirm what users would expect of this
> max_conflict_retention_duration option and it works as expected. IIUC
> users would want to use this option when they want to balance between
> the reliable update_deleted conflict detec
On Tue, Feb 4, 2025 at 10:30 PM Amit Kapila wrote:
>
> On Wed, Feb 5, 2025 at 6:00 AM Masahiko Sawada wrote:
> >
> > On Fri, Jan 31, 2025 at 9:07 PM Amit Kapila wrote:
> > >
> > > >
> > > > I was not sure of the point of
> > > > making the max_conflict_retention_duration a per-subscription
> > >
On Thu, Jan 23, 2025 at 5:17 PM Zhijie Hou (Fujitsu)
wrote:
>
I was reviewing v26 patch set and have some comments so far I reviewed
0001 so most of the comments/question are from this patch.
comments on v26-0001
1.
+ next_full_xid = ReadNextFullTransactionId();
+ epoch = EpochFromFullTransactio
On Wed, Feb 5, 2025 at 6:00 AM Masahiko Sawada wrote:
>
> On Fri, Jan 31, 2025 at 9:07 PM Amit Kapila wrote:
> >
> > >
> > > I was not sure of the point of
> > > making the max_conflict_retention_duration a per-subscription
> > > parameter.
> > >
> >
> > The idea is to keep it at the same level a
On Sat, Feb 1, 2025 at 10:37 AM Amit Kapila wrote:
>
> On Sat, Feb 1, 2025 at 2:54 AM Masahiko Sawada wrote:
> >
> > On Thu, Jan 30, 2025 at 10:39 PM Amit Kapila
> > wrote:
> > >
> > > On Fri, Jan 31, 2025 at 4:10 AM Masahiko Sawada
> > > wrote:
> > > >
> > > > I have one question about the 0
On Fri, Jan 31, 2025 at 9:07 PM Amit Kapila wrote:
>
> On Sat, Feb 1, 2025 at 2:54 AM Masahiko Sawada wrote:
> >
> > On Thu, Jan 30, 2025 at 10:39 PM Amit Kapila
> > wrote:
> > >
> > > On Fri, Jan 31, 2025 at 4:10 AM Masahiko Sawada
> > > wrote:
> > > >
> > > > I have one question about the 0
On Sat, Feb 1, 2025 at 2:54 AM Masahiko Sawada wrote:
>
> On Thu, Jan 30, 2025 at 10:39 PM Amit Kapila wrote:
> >
> > On Fri, Jan 31, 2025 at 4:10 AM Masahiko Sawada
> > wrote:
> > >
> > > I have one question about the 0004 patch; it implemented
> > > max_conflict_retntion_duration as a subscri
On Thu, Jan 30, 2025 at 10:39 PM Amit Kapila wrote:
>
> On Fri, Jan 31, 2025 at 4:10 AM Masahiko Sawada wrote:
> >
> > I have one question about the 0004 patch; it implemented
> > max_conflict_retntion_duration as a subscription parameter. But the
> > launcher invalidates the pg_conflict_detectio
On Fri, Jan 31, 2025 at 4:10 AM Masahiko Sawada wrote:
>
> I have one question about the 0004 patch; it implemented
> max_conflict_retntion_duration as a subscription parameter. But the
> launcher invalidates the pg_conflict_detection slot only if all
> subscriptions with retain_conflict_info stop
On Thu, Jan 23, 2025 at 3:47 AM Zhijie Hou (Fujitsu)
wrote:
>
> On Wednesday, January 22, 2025 7:54 PM Zhijie Hou (Fujitsu)
> wrote:
> > On Saturday, January 18, 2025 11:45 AM Zhijie Hou (Fujitsu)
> > wrote:
> > > I think invalidating the slot is OK and we could also let the apply
> > > worker
On Sat, Jan 18, 2025 at 9:15 AM Zhijie Hou (Fujitsu)
wrote:
>
>
> Here is the V24 patch set. I modified 0004 patch to implement the slot
> Invalidation part. Since the automatic recovery could be an optimization and
> the discussion is in progress, I didn't implement that part.
Few comments for p
On Wed, Jan 8, 2025 at 4:03 PM Masahiko Sawada wrote:
>
> On Wed, Jan 8, 2025 at 1:53 AM Amit Kapila wrote:
> > On Wed, Jan 8, 2025 at 3:02 PM Masahiko Sawada
> > wrote:
> > > On Thu, Dec 19, 2024 at 11:11 PM Nisha Moond
> > > wrote:
> > > > Here is further performance test analysis with v16
Dear hackers,
I've created a new script which simulates that user reduce the workload on the
publisher side. Attached zip file contains a script, execution log and pgbench
outputs. Experiments were done with v24 patch set.
Abstract
==
In this test the conflict slot could be invalidated as e
On Fri, Jan 17, 2025 at 1:37 PM Masahiko Sawada wrote:
>
> On Thu, Jan 16, 2025 at 2:02 AM Amit Kapila wrote:
> >
> > On Wed, Jan 15, 2025 at 2:20 PM Zhijie Hou (Fujitsu)
> > wrote:
> > >
> > > In the latest version, we implemented a simpler approach that allows the
> > > apply
> > > worker to
On Thu, Jan 16, 2025 at 2:02 AM Amit Kapila wrote:
>
> On Wed, Jan 15, 2025 at 2:20 PM Zhijie Hou (Fujitsu)
> wrote:
> >
> > In the latest version, we implemented a simpler approach that allows the
> > apply
> > worker to directly advance the oldest_nonremovable_xid if the waiting time
> > excee
On Wed, Jan 15, 2025 at 9:38 AM Amit Kapila wrote:
>
> On Wed, Jan 15, 2025 at 5:57 AM Masahiko Sawada wrote:
> >
> > Probably retaining dead tuples based on the time duration or its age
> > might be other solutions, it would increase a risk of not being able
> > to detect update_deleted conflict
On Thu, Jan 16, 2025 at 4:02 PM Amit Kapila wrote:
> On Thu, Jan 16, 2025 at 3:45 PM Dilip Kumar wrote:
> >
> > On Fri, Jan 3, 2025 at 4:31 PM Amit Kapila
> wrote:
> >>
> >> On Thu, Jan 2, 2025 at 2:57 PM vignesh C wrote:
> >> >
> >> > Conflict detection of truncated updates is detected as upd
On Thu, Jan 16, 2025 at 3:45 PM Dilip Kumar wrote:
>
> On Fri, Jan 3, 2025 at 4:31 PM Amit Kapila wrote:
>>
>> On Thu, Jan 2, 2025 at 2:57 PM vignesh C wrote:
>> >
>> > Conflict detection of truncated updates is detected as update_missing
>> > and deleted update is detected as update_deleted. I
On Fri, Jan 3, 2025 at 4:31 PM Amit Kapila wrote:
> On Thu, Jan 2, 2025 at 2:57 PM vignesh C wrote:
> >
> > Conflict detection of truncated updates is detected as update_missing
> > and deleted update is detected as update_deleted. I was not sure if
> > truncated updates should also be detected
On Wed, Jan 15, 2025 at 2:20 PM Zhijie Hou (Fujitsu)
wrote:
>
> In the latest version, we implemented a simpler approach that allows the apply
> worker to directly advance the oldest_nonremovable_xid if the waiting time
> exceeds the newly introduced option's limit. I've named this option
> "max_c
On Wed, Jan 15, 2025 at 5:57 AM Masahiko Sawada wrote:
>
> On Mon, Jan 13, 2025 at 8:39 PM Amit Kapila wrote:
> >
> > As of now, I can't think of a way to throttle the publisher when the
> > apply_worker lags. Basically, we need some way to throttle (reduce the
> > speed of backends) when the app
On Mon, Jan 13, 2025 at 8:39 PM Amit Kapila wrote:
>
> On Tue, Jan 14, 2025 at 7:14 AM Masahiko Sawada wrote:
> >
> > On Sun, Jan 12, 2025 at 10:36 PM Amit Kapila
> > wrote:
> > >
> > > I don't think we can avoid accumulating garbage especially when the
> > > workload on the publisher is more.
On Tue, Jan 14, 2025 at 7:14 AM Masahiko Sawada wrote:
>
> On Sun, Jan 12, 2025 at 10:36 PM Amit Kapila wrote:
> >
> > I don't think we can avoid accumulating garbage especially when the
> > workload on the publisher is more. Consider the current case being
> > discussed, on the publisher, we hav
On Sun, Jan 12, 2025 at 10:36 PM Amit Kapila wrote:
>
> On Fri, Jan 10, 2025 at 6:13 AM Masahiko Sawada wrote:
> >
> > 3. If the apply worker cannot catch up, it could enter to a bad loop;
> > the publisher sends huge amount of data -> the apply worker cannot
> > catch up -> it needs to wait for
Here are the performance test results and analysis with the recent patches.
Test Setup:
- Created Pub and Sub nodes with logical replication and below configurations.
autovacuum_naptime = '30s'
shared_buffers = '30GB'
max_wal_size = 20GB
min_wal_size = 10GB
track_commit_timestamp =
On Fri, Jan 10, 2025 at 6:13 AM Masahiko Sawada wrote:
>
> 3. If the apply worker cannot catch up, it could enter to a bad loop;
> the publisher sends huge amount of data -> the apply worker cannot
> catch up -> it needs to wait for a longer time to advance its
> oldest_nonremovable_xid -> more ga
On Friday, January 10, 2025 8:43 AM Masahiko Sawada
wrote:
Hi,
>
> On Wed, Jan 8, 2025 at 7:26 PM Zhijie Hou (Fujitsu)
> wrote:
> >
> > On Thursday, January 9, 2025 9:48 AM Masahiko Sawada
> wrote:
> >
> > Hi,
> >
> > >
> > > On Wed, Jan 8, 2025 at 3:00 AM Zhijie Hou (Fujitsu)
>
> > > wrote
On Wed, Jan 8, 2025 at 3:02 PM Masahiko Sawada wrote:
>
> On Thu, Dec 19, 2024 at 11:11 PM Nisha Moond wrote:
> > [3] Test with pgbench run on both publisher and subscriber.
> >
> > Test setup:
> > - Tests performed on pgHead + v16 patches
> > - Created a pub-sub replication system.
> > - Paramet
On Wed, Jan 8, 2025 at 7:26 PM Zhijie Hou (Fujitsu)
wrote:
>
> On Thursday, January 9, 2025 9:48 AM Masahiko Sawada
> wrote:
>
> Hi,
>
> >
> > On Wed, Jan 8, 2025 at 3:00 AM Zhijie Hou (Fujitsu)
> > wrote:
> > >
> > > On Wednesday, January 8, 2025 6:33 PM Masahiko Sawada
> > wrote:
> > >
> > >
On Wednesday, January 8, 2025 3:49 PM Nisha Moond
wrote:
>
> On Tue, Jan 7, 2025 at 6:04 PM Zhijie Hou (Fujitsu)
> wrote:
> >
> >
> > Attached the V19 patch which addressed comments in [1][2][3][4][5][6][7].
> >
>
> Here are a couple of initial review comments on v19 patch set:
>
> 1) The sub
On Wednesday, January 8, 2025 7:03 PM vignesh C wrote:
Hi,
> Consider a LR setup with retain_conflict_info=true for a table t1:
> Publisher:
> insert into t1 values(1);
> -- Have a open transaction before delete operation in subscriber begin;
>
> Subscriber:
> -- delete the record that was repl
On Wed, Jan 8, 2025 at 2:24 PM Amit Kapila wrote:
>
> On Wed, Jan 8, 2025 at 2:15 PM Masahiko Sawada wrote:
> >
> > On Tue, Jan 7, 2025 at 2:49 AM Amit Kapila wrote:
> > > >
> > > > We thought of another approach, which is to create/drop this slot first
> > > > as
> > > > soon as one enables/di
On Thursday, January 9, 2025 9:48 AM Masahiko Sawada
Hi,
>
> On Wed, Jan 8, 2025 at 3:00 AM Zhijie Hou (Fujitsu)
> wrote:
> >
> > On Wednesday, January 8, 2025 6:33 PM Masahiko Sawada
> wrote:
> >
> > Hi,
> >
> > > On Wed, Jan 8, 2025 at 1:53 AM Amit Kapila
> > > wrote:
> > > > On Wed, Jan 8
On Thursday, January 9, 2025 9:48 AM Masahiko Sawada
wrote:
Hi,
>
> On Wed, Jan 8, 2025 at 3:00 AM Zhijie Hou (Fujitsu)
> wrote:
> >
> > On Wednesday, January 8, 2025 6:33 PM Masahiko Sawada
> wrote:
> >
> > Hi,
> >
> > > On Wed, Jan 8, 2025 at 1:53 AM Amit Kapila
> > > wrote:
> > > > On We
On Wed, Jan 8, 2025 at 3:00 AM Zhijie Hou (Fujitsu)
wrote:
>
> On Wednesday, January 8, 2025 6:33 PM Masahiko Sawada
> wrote:
>
> Hi,
>
> > On Wed, Jan 8, 2025 at 1:53 AM Amit Kapila
> > wrote:
> > > On Wed, Jan 8, 2025 at 3:02 PM Masahiko Sawada
> > wrote:
> > > >
> > > > On Thu, Dec 19, 2024
On Tue, 7 Jan 2025 at 18:04, Zhijie Hou (Fujitsu)
wrote:
>
> On Friday, January 3, 2025 1:53 PM Amit Kapila
> wrote:
> >
> > On Wed, Dec 25, 2024 at 8:13 AM Zhijie Hou (Fujitsu)
> > wrote:
> > >
> > > Attach the new version patch set which addressed all other comments.
> > >
> >
> > Some more m
On Wednesday, January 8, 2025 6:33 PM Masahiko Sawada
wrote:
Hi,
> On Wed, Jan 8, 2025 at 1:53 AM Amit Kapila
> wrote:
> > On Wed, Jan 8, 2025 at 3:02 PM Masahiko Sawada
> wrote:
> > >
> > > On Thu, Dec 19, 2024 at 11:11 PM Nisha Moond
> wrote:
> > > >
> > > >
> > > > [3] Test with pgbench r
On Wed, Jan 8, 2025 at 1:53 AM Amit Kapila wrote:
>
> On Wed, Jan 8, 2025 at 3:02 PM Masahiko Sawada wrote:
> >
> > On Thu, Dec 19, 2024 at 11:11 PM Nisha Moond
> > wrote:
> > >
> > > Here is further performance test analysis with v16 patch-set.
> > >
> > >
> > > In the test scenarios already s
On Wed, Jan 8, 2025 at 3:02 PM Masahiko Sawada wrote:
>
> On Thu, Dec 19, 2024 at 11:11 PM Nisha Moond wrote:
> >
> > Here is further performance test analysis with v16 patch-set.
> >
> >
> > In the test scenarios already shared on -hackers [1], where pgbench was run
> > only on the publisher no
On Thu, Dec 19, 2024 at 11:11 PM Nisha Moond wrote:
>
> Here is further performance test analysis with v16 patch-set.
>
>
> In the test scenarios already shared on -hackers [1], where pgbench was run
> only on the publisher node in a pub-sub setup, no performance degradation was
> observed on ei
On Wed, Jan 8, 2025 at 2:15 PM Masahiko Sawada wrote:
>
> On Tue, Jan 7, 2025 at 2:49 AM Amit Kapila wrote:
> > >
> > > We thought of another approach, which is to create/drop this slot first as
> > > soon as one enables/disables detect_update_deleted (E.g. create/drop slot
> > > during DDL). But
On Tue, Jan 7, 2025 at 2:49 AM Amit Kapila wrote:
>
> On Mon, Jan 6, 2025 at 4:52 PM Zhijie Hou (Fujitsu)
> wrote:
> >
> > On Friday, January 3, 2025 2:36 PM Masahiko Sawada
> > wrote:
> >
> > >
> > > + /*
> > > +* The changes made by this and later transactions are still
> > > no
On Tue, Jan 7, 2025 at 6:04 PM Zhijie Hou (Fujitsu)
wrote:
>
>
> Attached the V19 patch which addressed comments in [1][2][3][4][5][6][7].
>
Here are a couple of initial review comments on v19 patch set:
1) The subscription option 'retain_conflict_info' remains set to
"true" for a subscription e
On Tue, 7 Jan 2025 at 18:04, Zhijie Hou (Fujitsu)
wrote:
>
> On Friday, January 3, 2025 1:53 PM Amit Kapila
> wrote:
> >
> > On Wed, Dec 25, 2024 at 8:13 AM Zhijie Hou (Fujitsu)
> > wrote:
> > >
> > > Attach the new version patch set which addressed all other comments.
> > >
> >
> > Some more m
On Thursday, January 2, 2025 2:30 PM Amit Kapila
wrote:
>
> Sounds reasonable but OTOH, all other places that create physical
> slots (which we are doing here) don't use this trick. So, don't they
> need similar reliability?
I have not figured the reason for existing physical slots' handling,
b
On Thursday, January 2, 2025 6:34 PM vignesh C wrote:
>
> Few suggestions:
> 1) If we have a subscription with detect_update_deleted option and we
> try to upgrade it with default settings(in case dba forgot to set
> track_commit_timestamp), the upgrade will fail after doing a lot of
> steps like
Dear Amit, Hou,
> BTW, it is not clear how retaining dead tuples will help the detection
> update_origin_differs. Will it happen when the tuple is inserted or
> updated on the subscriber and then when we try to update the same
> tuple due to remote update, the commit_ts information of the xact is
On Wed, 25 Dec 2024 at 08:13, Zhijie Hou (Fujitsu)
wrote:
>
> On Monday, December 23, 2024 2:15 PM Kuroda, Hayato/黒田 隼人
> wrote:
> >
> > Dear Hou,
> >
> > Thanks for updating the patch. Few comments:
>
> Thanks for the comments!
>
> > 02. ErrorOnReservedSlotName()
> >
> > Currently the function
On Mon, Jan 6, 2025 at 4:52 PM Zhijie Hou (Fujitsu)
wrote:
>
> On Friday, January 3, 2025 2:36 PM Masahiko Sawada
> wrote:
>
> >
> > + /*
> > +* The changes made by this and later transactions are still
> > non-removable
> > +* to allow for the detection of update_deleted c
On Tuesday, January 7, 2025 3:05 PM Masahiko Sawada
wrote:
Hi,
> On Mon, Jan 6, 2025 at 10:40 PM Zhijie Hou (Fujitsu)
> wrote:
> >
> > On Tuesday, January 7, 2025 2:00 PM Masahiko Sawada
> wrote:
> >
> > Hi,
> >
> > >
> > > On Mon, Jan 6, 2025 at 3:22 AM Zhijie Hou (Fujitsu)
>
> > > wrote:
>
On Fri, Jan 3, 2025 at 11:22 AM Amit Kapila wrote:
>
> 5.
> +
> + id="sql-createsubscription-params-with-detect-update-deleted">
> +detect_update_deleted
> (boolean)
> +
> +
> + Specifies whether the detection of linkend="conflict-update-deleted"/>
> +
On Mon, Jan 6, 2025 at 10:40 PM Zhijie Hou (Fujitsu)
wrote:
>
> On Tuesday, January 7, 2025 2:00 PM Masahiko Sawada
> wrote:
>
> Hi,
>
> >
> > On Mon, Jan 6, 2025 at 3:22 AM Zhijie Hou (Fujitsu)
> > wrote:
> > >
> > > On Friday, January 3, 2025 2:36 PM Masahiko Sawada
> > wrote:
> > >
> > > >
On Tuesday, January 7, 2025 2:00 PM Masahiko Sawada
wrote:
Hi,
>
> On Mon, Jan 6, 2025 at 3:22 AM Zhijie Hou (Fujitsu)
> wrote:
> >
> > On Friday, January 3, 2025 2:36 PM Masahiko Sawada
> wrote:
> >
> > >
> > > I have one comment on the 0001 patch:
> >
> > Thanks for the comments!
> >
> > >
On Mon, Jan 6, 2025 at 3:22 AM Zhijie Hou (Fujitsu)
wrote:
>
> On Friday, January 3, 2025 2:36 PM Masahiko Sawada
> wrote:
>
> Hi,
>
> >
> > I have one comment on the 0001 patch:
>
> Thanks for the comments!
>
> >
> > + /*
> > +* The changes made by this and later transactions are
On Mon, Jan 6, 2025 at 4:52 PM Zhijie Hou (Fujitsu)
wrote:
>
> On Friday, January 3, 2025 2:36 PM Masahiko Sawada
> wrote:
>
> Hi,
>
> >
> > I have one comment on the 0001 patch:
>
> Thanks for the comments!
>
> >
> > + /*
> > +* The changes made by this and later transactions are
On Friday, January 3, 2025 2:36 PM Masahiko Sawada
wrote:
Hi,
>
> I have one comment on the 0001 patch:
Thanks for the comments!
>
> + /*
> +* The changes made by this and later transactions are still
> non-removable
> +* to allow for the detection of update_deleted co
On Fri, Dec 20, 2024 at 12:41 PM Nisha Moond wrote:
>
> In the test scenarios already shared on -hackers [1], where pgbench was run
> only on the publisher node in a pub-sub setup, no performance degradation was
> observed on either node.
>
>
>
> In contrast, when pgbench was run only on the sub
On Thu, Jan 2, 2025 at 2:57 PM vignesh C wrote:
>
> Conflict detection of truncated updates is detected as update_missing
> and deleted update is detected as update_deleted. I was not sure if
> truncated updates should also be detected as update_deleted, as the
> document says truncate operation i
On Fri, Jan 3, 2025 at 2:34 PM vignesh C wrote:
>
> Few comments:
> 1) In case there are no logical replication workers, the launcher
> process just logs a warning "out of logical replication worker slots"
> and continues. Whereas in case of "pg_conflict_detection" replication
> slot creation laun
On Fri, Jan 3, 2025 at 12:06 PM Masahiko Sawada wrote:
>
> I have one comment on the 0001 patch:
>
> + /*
> +* The changes made by this and later transactions are still
> non-removable
> +* to allow for the detection of update_deleted conflicts when
> applying
> +* c
On Wed, 25 Dec 2024 at 08:13, Zhijie Hou (Fujitsu)
wrote:
>
> On Monday, December 23, 2024 2:15 PM Kuroda, Hayato/黒田 隼人
> wrote:
> >
> > Dear Hou,
> >
> > Thanks for updating the patch. Few comments:
>
> Thanks for the comments!
>
> > 02. ErrorOnReservedSlotName()
> >
> > Currently the function
On Tue, Dec 24, 2024 at 6:43 PM Zhijie Hou (Fujitsu)
wrote:
>
> On Monday, December 23, 2024 2:15 PM Kuroda, Hayato/黒田 隼人
> wrote:
> >
> > Dear Hou,
> >
> > Thanks for updating the patch. Few comments:
>
> Thanks for the comments!
>
> > 02. ErrorOnReservedSlotName()
> >
> > Currently the functi
On Wed, Dec 25, 2024 at 8:13 AM Zhijie Hou (Fujitsu)
wrote:
>
> Attach the new version patch set which addressed all other comments.
>
Some more miscellaneous comments:
=
1.
@@ -1431,9 +1431,9 @@ RecordTransactionCommit(void)
* modifying it. This makes checkpoint's
On Wed, 25 Dec 2024 at 08:13, Zhijie Hou (Fujitsu)
wrote:
>
> On Monday, December 23, 2024 2:15 PM Kuroda, Hayato/黒田 隼人
> wrote:
> >
> > Dear Hou,
> >
> > Thanks for updating the patch. Few comments:
>
> Thanks for the comments!
>
> > 02. ErrorOnReservedSlotName()
> >
> > Currently the function
On Wed, 25 Dec 2024 at 08:13, Zhijie Hou (Fujitsu)
wrote:
>
> On Monday, December 23, 2024 2:15 PM Kuroda, Hayato/黒田 隼人
> wrote:
> >
> > Dear Hou,
> >
> > Thanks for updating the patch. Few comments:
>
> Thanks for the comments!
>
> > 02. ErrorOnReservedSlotName()
> >
> > Currently the function
On Wed, Dec 25, 2024 at 8:13 AM Zhijie Hou (Fujitsu)
wrote:
>
> On Monday, December 23, 2024 2:15 PM Kuroda, Hayato/黒田 隼人
> wrote:
> >
> > Dear Hou,
> >
> > Thanks for updating the patch. Few comments:
>
> Thanks for the comments!
>
> > 02. ErrorOnReservedSlotName()
> >
> > Currently the functi
On Wed, Dec 25, 2024 at 8:13 AM Zhijie Hou (Fujitsu)
wrote:
>
> Attach the new version patch set which addressed all other comments.
>
Review comments on 0001 and 0002
=
1.
/*
+ * Reset all data fields except those used to determine the timing for the
+ * next round of
On Thu, Dec 19, 2024 at 4:34 PM Zhijie Hou (Fujitsu)
wrote:
>
> On Sunday, December 15, 2024 9:39 AM Amit Kapila
> wrote:
> >
>
> >
> > 5. The apply worker needs to at least twice get the publisher status
> > message to
> > advance oldest_nonremovable_xid once. It then uses the remote_lsn of th
Dear Hou,
Thanks for updating the patch. Few comments:
01. worker.c
```
+/*
+ * The minimum (100ms) and maximum (3 minutes) intervals for advancing
+ * non-removable transaction IDs.
+ */
+#define MIN_XID_ADVANCEMENT_INTERVAL 100
+#define MAX_XID_ADVANCEMENT_INTERVAL 18L
```
Since the max_i
Here is further performance test analysis with v16 patch-set.
In the test scenarios already shared on -hackers [1], where pgbench was run
only on the publisher node in a pub-sub setup, no performance degradation
was observed on either node.
In contrast, when pgbench was run only on the subscri
On Tue, Dec 17, 2024 at 8:54 AM Zhijie Hou (Fujitsu)
wrote:
>
> On Monday, December 16, 2024 7:21 PM Dilip Kumar
> wrote:
>
> > So IIUC in step 2) we send the message and get the list of all the
> > transactions which are in the commit phase? What do you exactly mean by a
> > transaction which i
On Monday, December 16, 2024 7:21 PM Dilip Kumar wrote:
Hi,
>
> On Wed, Dec 11, 2024 at 2:32 PM Zhijie Hou (Fujitsu)
> wrote:
> >
> > Attach the V16 patch set which addressed above comments.
> >
> > There is a new 0002 patch where I tried to dynamically adjust the interval
> > for
> > advanci
On Wed, Dec 11, 2024 at 2:32 PM Zhijie Hou (Fujitsu)
wrote:
>
> Attach the V16 patch set which addressed above comments.
>
> There is a new 0002 patch where I tried to dynamically adjust the interval for
> advancing the transaction ID. Instead of always waiting for
> wal_receiver_status_interval,
On Wed, Dec 11, 2024 at 2:32 PM Zhijie Hou (Fujitsu)
wrote:
>
> Attach the V16 patch set which addressed above comments.
>
Review Comments
1.
+/*
+ * Workhorse for the RCI_WAIT_FOR_LOCAL_FLUSH phase.
+ */
+static void
+wait_for_local_flush(RetainConflictInfoData *data)
+{
...
+ /
Dear hackers,
I did some benchmarks with the patch. More detail, a pub-sub replication system
was built and TPS was measured on the subscriber. Results were shown that the
performance can be degraded if the wal_receiver_status_interval is long.
This is expected behavior because the patch retains m
On Wednesday, December 11, 2024 1:06 PM Amit Kapila
wrote:
> On Fri, Dec 6, 2024 at 1:28 PM Zhijie Hou (Fujitsu)
> wrote:
> >
> > On Thursday, December 5, 2024 6:00 PM Amit Kapila
> wrote:
> > >
> > >
> > > A few more comments:
> > > 1.
> > > +static void
> > > +wait_for_local_flush(RetainConfl
On Fri, Dec 6, 2024 at 1:28 PM Zhijie Hou (Fujitsu)
wrote:
>
> On Thursday, December 5, 2024 6:00 PM Amit Kapila
> wrote:
> >
> >
> > A few more comments:
> > 1.
> > +static void
> > +wait_for_local_flush(RetainConflictInfoData *data)
> > {
> > ...
> > +
> > + data->phase = RCI_GET_CANDIDATE_XID
Here are the test steps and analysis for epoch-related handling
(Tested with v15 Patch-Set).
In the 'update_deleted' detection design, the launcher process
compares XIDs to track minimum XIDs, and the apply workers maintain
the oldest running XIDs. The launcher also requests publisher status
at re
On Mon, Dec 9, 2024 at 3:20 PM Nisha Moond wrote:
>
> Here is a summary of tests targeted to the Publisher node in a
> Publisher-Subscriber setup.
> (All tests done with v14 patch-set)
>
>
> Performance Tests:
>
> Test machine details:
> In
On Wed, Dec 4, 2024 at 4:29 PM Amit Kapila wrote:
>
> 1.
> + if (can_advance_nonremovable_xid(&data, last_recv_timestamp))
> + maybe_advance_nonremovable_xid(&data);
>
> In can_advance_nonremovable_xid(), we determine whether to advance the
> oldest xid based on 'last_recv_timestamp' and then agai
On Tue, Dec 3, 2024 at 9:17 AM Zhijie Hou (Fujitsu)
wrote:
>
> Also, I removed a change in wait_for_local_flush() which was mis-added in
> V13_2
> patch.
>
1.
+ if (can_advance_nonremovable_xid(&data, last_recv_timestamp))
+ maybe_advance_nonremovable_xid(&data);
In can_advance_nonremovable_xid
On Friday, November 29, 2024 6:35 PM Kuroda, Hayato/黒田 隼人
wrote:
>
> Dear Hou,
>
> Thanks for updating the patch! Here are my comments mainly for 0001.
Thanks for the comments!
>
> 02. maybe_advance_nonremovable_xid
>
> ```
> +case RCI_REQUEST_PUBLISHER_STATUS:
> +reques
On Fri, Nov 29, 2024 at 4:05 PM Hayato Kuroda (Fujitsu)
wrote:
>
> 02. maybe_advance_nonremovable_xid
>
> ```
> +case RCI_REQUEST_PUBLISHER_STATUS:
> +request_publisher_status(data);
> +break;
> ```
>
> I think the part is not reachable because the transit
> RCI_REQ
On Fri, Nov 29, 2024 at 4:05 PM Hayato Kuroda (Fujitsu)
wrote:
>
> 07. wait_for_publisher_status
>
> I think all calculations and checking in the function can be done even on the
> walsender. Based on this, I come up with an idea to reduce the message size:
> walsender can just send a status (bool
Dear Hou,
Thanks for updating the patch! Here are my comments mainly for 0001.
01. protocol.sgml
I think the ordering of attributes in "Primary status update" seems not correct.
The second entry is LSN, not the oldest running xid.
02. maybe_advance_nonremovable_xid
```
+case RCI_REQUES
On Fri, Nov 29, 2024 at 7:54 AM Zhijie Hou (Fujitsu)
wrote:
>
> It is possible to reach here if user creates a subscription with
> (connect=false,detect_update_deleted=true), in which case we could not check
> it
> during creation. But I agree that it would be better to report an ERROR here,
> so
On Tue, Nov 26, 2024 at 1:50 PM Zhijie Hou (Fujitsu)
wrote:
>
Few comments on the latest 0001 patch:
1.
+ * - RCI_REQUEST_PUBLISHER_STATUS:
+ * Send a message to the walsender requesting the publisher status, which
+ * includes the latest WAL write position and information about running
+ *
On Thu, Nov 21, 2024 at 3:03 PM Zhijie Hou (Fujitsu)
wrote:
>
> Attach the V10 patch set which addressed above comments
> and fixed a CFbot warning due to un-initialized variable.
>
We should make the v10_2-0001* as the first main patch for review till
we have a consensus to resolve LSN<->Timesta
1 - 100 of 150 matches
Mail list logo