Re: Conflict detection for update_deleted in logical replication

2025-04-29 Thread shveta malik
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

Re: Conflict detection for update_deleted in logical replication

2025-04-25 Thread shveta malik
> > 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

Re: Conflict detection for update_deleted in logical replication

2025-04-24 Thread shveta malik
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. > > +

Re: Conflict detection for update_deleted in logical replication

2025-04-17 Thread shveta malik
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 +

Re: Conflict detection for update_deleted in logical replication

2025-04-16 Thread Nisha Moond
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

Re: Conflict detection for update_deleted in logical replication

2025-04-15 Thread shveta malik
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

RE: Conflict detection for update_deleted in logical replication

2025-04-15 Thread Hayato Kuroda (Fujitsu)
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

Re: Conflict detection for update_deleted in logical replication

2025-03-12 Thread vignesh C
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

Re: Conflict detection for update_deleted in logical replication

2025-02-10 Thread Dilip Kumar
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

Re: Conflict detection for update_deleted in logical replication

2025-02-10 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2025-02-09 Thread Dilip Kumar
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

Re: Conflict detection for update_deleted in logical replication

2025-02-07 Thread Masahiko Sawada
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

Re: Conflict detection for update_deleted in logical replication

2025-02-06 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2025-02-06 Thread Masahiko Sawada
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 > > >

Re: Conflict detection for update_deleted in logical replication

2025-02-04 Thread Dilip Kumar
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

Re: Conflict detection for update_deleted in logical replication

2025-02-04 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2025-02-04 Thread Dilip Kumar
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

Re: Conflict detection for update_deleted in logical replication

2025-02-04 Thread Masahiko Sawada
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

Re: Conflict detection for update_deleted in logical replication

2025-01-31 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2025-01-31 Thread Masahiko Sawada
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

Re: Conflict detection for update_deleted in logical replication

2025-01-30 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2025-01-30 Thread Masahiko Sawada
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

Re: Conflict detection for update_deleted in logical replication

2025-01-22 Thread Nisha Moond
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

Re: Conflict detection for update_deleted in logical replication

2025-01-20 Thread Nisha Moond
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

RE: Conflict detection for update_deleted in logical replication

2025-01-19 Thread Hayato Kuroda (Fujitsu)
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

Re: Conflict detection for update_deleted in logical replication

2025-01-17 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2025-01-17 Thread Masahiko Sawada
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

Re: Conflict detection for update_deleted in logical replication

2025-01-16 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2025-01-16 Thread Dilip Kumar
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

Re: Conflict detection for update_deleted in logical replication

2025-01-16 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2025-01-16 Thread Dilip Kumar
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

Re: Conflict detection for update_deleted in logical replication

2025-01-16 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2025-01-14 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2025-01-14 Thread Masahiko Sawada
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.

Re: Conflict detection for update_deleted in logical replication

2025-01-13 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2025-01-13 Thread Masahiko Sawada
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

Re: Conflict detection for update_deleted in logical replication

2025-01-13 Thread Nisha Moond
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 =

Re: Conflict detection for update_deleted in logical replication

2025-01-12 Thread Amit Kapila
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

RE: Conflict detection for update_deleted in logical replication

2025-01-10 Thread Zhijie Hou (Fujitsu)
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

Re: Conflict detection for update_deleted in logical replication

2025-01-10 Thread Nisha Moond
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

Re: Conflict detection for update_deleted in logical replication

2025-01-09 Thread Masahiko Sawada
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: > > > > > >

RE: Conflict detection for update_deleted in logical replication

2025-01-09 Thread Zhijie Hou (Fujitsu)
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

RE: Conflict detection for update_deleted in logical replication

2025-01-09 Thread Zhijie Hou (Fujitsu)
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

Re: Conflict detection for update_deleted in logical replication

2025-01-08 Thread Amit Kapila
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

RE: Conflict detection for update_deleted in logical replication

2025-01-08 Thread Zhijie Hou (Fujitsu)
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

RE: Conflict detection for update_deleted in logical replication

2025-01-08 Thread Zhijie Hou (Fujitsu)
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

Re: Conflict detection for update_deleted in logical replication

2025-01-08 Thread Masahiko Sawada
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

Re: Conflict detection for update_deleted in logical replication

2025-01-08 Thread vignesh C
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

RE: Conflict detection for update_deleted in logical replication

2025-01-08 Thread Zhijie Hou (Fujitsu)
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

Re: Conflict detection for update_deleted in logical replication

2025-01-08 Thread Masahiko Sawada
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

Re: Conflict detection for update_deleted in logical replication

2025-01-08 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2025-01-08 Thread Masahiko Sawada
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

Re: Conflict detection for update_deleted in logical replication

2025-01-08 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2025-01-08 Thread Masahiko Sawada
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

Re: Conflict detection for update_deleted in logical replication

2025-01-07 Thread Nisha Moond
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

Re: Conflict detection for update_deleted in logical replication

2025-01-07 Thread vignesh C
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

RE: Conflict detection for update_deleted in logical replication

2025-01-07 Thread Zhijie Hou (Fujitsu)
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

RE: Conflict detection for update_deleted in logical replication

2025-01-07 Thread Zhijie Hou (Fujitsu)
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

RE: Conflict detection for update_deleted in logical replication

2025-01-07 Thread Hayato Kuroda (Fujitsu)
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

Re: Conflict detection for update_deleted in logical replication

2025-01-07 Thread vignesh 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

Re: Conflict detection for update_deleted in logical replication

2025-01-07 Thread Amit Kapila
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

RE: Conflict detection for update_deleted in logical replication

2025-01-07 Thread Zhijie Hou (Fujitsu)
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: >

Re: Conflict detection for update_deleted in logical replication

2025-01-07 Thread Amit Kapila
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"/> > +

Re: Conflict detection for update_deleted in logical replication

2025-01-06 Thread Masahiko Sawada
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: > > > > > > >

RE: Conflict detection for update_deleted in logical replication

2025-01-06 Thread Zhijie Hou (Fujitsu)
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! > > > > >

Re: Conflict detection for update_deleted in logical replication

2025-01-06 Thread Masahiko Sawada
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

Re: Conflict detection for update_deleted in logical replication

2025-01-06 Thread Nisha Moond
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

RE: Conflict detection for update_deleted in logical replication

2025-01-06 Thread Zhijie Hou (Fujitsu)
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

Re: Conflict detection for update_deleted in logical replication

2025-01-05 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2025-01-03 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2025-01-03 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2025-01-03 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2025-01-03 Thread vignesh 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

Re: Conflict detection for update_deleted in logical replication

2025-01-02 Thread Masahiko Sawada
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

Re: Conflict detection for update_deleted in logical replication

2025-01-02 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2025-01-02 Thread vignesh 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

Re: Conflict detection for update_deleted in logical replication

2025-01-02 Thread vignesh 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

Re: Conflict detection for update_deleted in logical replication

2025-01-01 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2025-01-01 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2024-12-31 Thread Amit Kapila
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

RE: Conflict detection for update_deleted in logical replication

2024-12-22 Thread Hayato Kuroda (Fujitsu)
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

Re: Conflict detection for update_deleted in logical replication

2024-12-19 Thread Nisha Moond
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

Re: Conflict detection for update_deleted in logical replication

2024-12-17 Thread Dilip Kumar
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

RE: Conflict detection for update_deleted in logical replication

2024-12-16 Thread Zhijie Hou (Fujitsu)
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

Re: Conflict detection for update_deleted in logical replication

2024-12-16 Thread Dilip Kumar
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,

Re: Conflict detection for update_deleted in logical replication

2024-12-14 Thread Amit Kapila
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) +{ ... + /

RE: Conflict detection for update_deleted in logical replication

2024-12-11 Thread Hayato Kuroda (Fujitsu)
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

RE: Conflict detection for update_deleted in logical replication

2024-12-10 Thread Zhijie Hou (Fujitsu)
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

Re: Conflict detection for update_deleted in logical replication

2024-12-10 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2024-12-10 Thread Nisha Moond
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

Re: Conflict detection for update_deleted in logical replication

2024-12-09 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2024-12-05 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2024-12-04 Thread Amit Kapila
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

RE: Conflict detection for update_deleted in logical replication

2024-12-02 Thread Zhijie Hou (Fujitsu)
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

Re: Conflict detection for update_deleted in logical replication

2024-12-01 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2024-11-29 Thread Amit Kapila
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

RE: Conflict detection for update_deleted in logical replication

2024-11-29 Thread Hayato Kuroda (Fujitsu)
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

Re: Conflict detection for update_deleted in logical replication

2024-11-29 Thread Amit Kapila
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

Re: Conflict detection for update_deleted in logical replication

2024-11-27 Thread Amit Kapila
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 + *

Re: Conflict detection for update_deleted in logical replication

2024-11-25 Thread Amit Kapila
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   2   >