On Thu, Aug 8, 2024 at 6:08 PM Amit Kapila wrote:
>
> On Thu, Aug 8, 2024 at 3:41 PM Zhijie Hou (Fujitsu)
> wrote:
> >
> > On Thursday, August 8, 2024 6:01 PM shveta malik
> > wrote:
> > >
> > > On Thu, Aug 8, 2024 at 12:03 PM Amit Kapila
> >
On Wed, Aug 7, 2024 at 2:00 PM vignesh C wrote:
>
> On Wed, 7 Aug 2024 at 08:09, Amit Kapila wrote:
> >
> > On Tue, Aug 6, 2024 at 5:13 PM vignesh C wrote:
> > >
> > > On Mon, 5 Aug 2024 at 18:05, shveta malik wrote:
> > > >
> >
On Fri, Aug 9, 2024 at 2:39 PM Hayato Kuroda (Fujitsu)
wrote:
>
> Dear Amit, Shveta, Hou,
>
> Thanks for giving many comments! I've updated the patch.
>
> > @@ -4409,6 +4409,14 @@ start_apply(XLogRecPtr origin_startpos)
> > }
> > PG_CATCH();
> > {
> > + /*
> > + * Reset the origin data to pr
On Mon, Aug 12, 2024 at 3:36 PM shveta malik wrote:
>
> On Fri, Aug 9, 2024 at 2:39 PM Hayato Kuroda (Fujitsu)
> wrote:
> >
> > Dear Amit, Shveta, Hou,
> >
> > Thanks for giving many comments! I've updated the patch.
> >
> > > @@ -4409
On Tue, Aug 13, 2024 at 9:48 AM Amit Kapila wrote:
>
> On Mon, Aug 12, 2024 at 3:37 PM shveta malik wrote:
> >
> > On Fri, Aug 9, 2024 at 2:39 PM Hayato Kuroda (Fujitsu)
> > wrote:
> > >
> > >
> > > > + /*
> > > > + *
On Tue, Aug 13, 2024 at 9:48 AM Amit Kapila wrote:
>
> BTW, this needs to be backpatched till 16 when it has been introduced
> by the parallel apply feature as part of commit 216a7848. So, can we
> test this patch in back-branches as well?
>
I was able to reproduce the problem on REL_16_STABLE an
On Thu, Aug 15, 2024 at 12:47 PM Zhijie Hou (Fujitsu)
wrote:
>
> Thanks. I have checked and merged the changes. Here is the V15 patch
> which addressed above comments.
Thanks for the patch. Please find few comments and queries:
1)
For various conflicts , we have these in Logs:
Replica identity (
On Fri, Aug 16, 2024 at 10:46 AM shveta malik wrote:
>
> 3)
> For update_exists(), we dump:
> Key (a, b)=(2, 1)
>
> For delete_missing, update_missing, update_differ, we dump:
> Replica identity (a, b)=(2, 1).
>
> For update_exists as well, shouldn't we dump '
On Fri, Aug 16, 2024 at 12:19 PM Amit Kapila wrote:
>
> On Fri, Aug 16, 2024 at 11:48 AM shveta malik wrote:
> >
> > On Fri, Aug 16, 2024 at 10:46 AM shveta malik
> > wrote:
> > >
> > > 3)
> > > For update_exists(), we dump:
>
On Sun, Aug 18, 2024 at 2:27 PM Zhijie Hou (Fujitsu)
wrote:
>
> Attach the V16 patch which addressed the comments we agreed on.
> I will add a doc patch to explain the log format after the 0001 is RFC.
>
Thank You for addressing comments. Please see this scenario:
create table tab1(pk int primar
On Mon, Aug 19, 2024 at 9:07 AM shveta malik wrote:
>
> On Sun, Aug 18, 2024 at 2:27 PM Zhijie Hou (Fujitsu)
> wrote:
> >
> > Attach the V16 patch which addressed the comments we agreed on.
> > I will add a doc patch to explain the log format after the 0001 is
On Mon, Aug 19, 2024 at 11:37 AM Amit Kapila wrote:
>
> On Mon, Aug 19, 2024 at 9:08 AM shveta malik wrote:
> >
> > On Sun, Aug 18, 2024 at 2:27 PM Zhijie Hou (Fujitsu)
> > wrote:
> > >
> > > Attach the V16 patch which addressed the comments we agreed o
On Mon, Aug 19, 2024 at 12:09 PM Amit Kapila wrote:
>
> On Mon, Aug 19, 2024 at 11:54 AM shveta malik wrote:
> >
> > On Mon, Aug 19, 2024 at 11:37 AM Amit Kapila
> > wrote:
> > >
> > > On Mon, Aug 19, 2024 at 9:08 AM shveta malik
> > > wro
On Mon, Aug 19, 2024 at 12:32 PM Zhijie Hou (Fujitsu)
wrote:
>
>
> Thanks for reporting the bug. I have fixed it and ran pgindent in V17 patch.
> I also adjusted few comments and fixed a typo.
>
Thanks for the patch. Re-tested it, all scenarios seem to work well now.
I see that this version has
On Tue, Aug 20, 2024 at 11:36 AM Amit Kapila wrote:
>
> On Wed, Aug 14, 2024 at 10:26 AM shveta malik wrote:
> >
> > >
> > > Thanks for the detailed analysis. I agree with your analysis that we
> > > need to reset the origin information for the shutdow
On Tue, Oct 8, 2024 at 3:12 PM Nisha Moond wrote:
>
I have not started reviewing v15 yet, but here are few comments for
v14-patch003:
1)
In apply_handle_update_internal(), I see that
FindReplTupleInLocalRel() used to lock the row to be updated in
exclusive mode, but now since we are avoiding thi
On Wed, Oct 9, 2024 at 8:58 AM shveta malik wrote:
>
> On Tue, Oct 8, 2024 at 3:12 PM Nisha Moond wrote:
> >
>
Please find few comments on v14-patch004:
patch004:
1)
GetConflictResolver currently errors out when the resolver is
last_update_wins and track_commit_timestamp is dis
On Mon, Oct 7, 2024 at 4:03 PM vignesh C wrote:
>
With parallel streaming as default, do you think there is a need to
increase the default for 'max_logical_replication_workers' as IIUC
parallel workers are taken from the same pool.
thanks
Shveta
On Mon, Sep 30, 2024 at 11:04 AM Peter Smith wrote:
>
> On Mon, Sep 30, 2024 at 2:27 PM shveta malik wrote:
> >
> > On Fri, Sep 27, 2024 at 1:00 PM Peter Smith wrote:
> ...
> > >
> > > 13. General - ordering of conflict_type.
> > >
> > >
On Fri, Sep 27, 2024 at 2:33 PM shveta malik wrote:
>
> On Fri, Sep 27, 2024 at 10:44 AM shveta malik wrote:
> >
> > > >
> > > > Thanks for the review.
> > > > Here is the v14 patch-set fixing review comments in [1] and [2].
> > > >
>
On Fri, Sep 27, 2024 at 1:00 PM Peter Smith wrote:
>
> Here are some review comments for v14-0001.
>
> This is a WIP, but here are my comments for all the SGML parts.
>
> (There will be some overlap here with comments already posted by Shveta)
>
> ==
> 1. file modes after applying the patch
>
On Fri, Oct 18, 2024 at 4:30 PM Zhijie Hou (Fujitsu)
wrote:
>
> On Wednesday, October 9, 2024 2:34 PM shveta malik
> wrote:
> >
> > On Wed, Oct 9, 2024 at 8:58 AM shveta malik
> > wrote:
> > >
> > > On Tue, Oct 8, 2024 at 3:12 PM Nisha Moond
>
On Tue, Oct 8, 2024 at 3:38 PM Amit Kapila wrote:
>
> On Tue, Oct 8, 2024 at 2:25 PM shveta malik wrote:
> >
> > On Mon, Oct 7, 2024 at 4:03 PM vignesh C wrote:
> > >
> >
> > With parallel streaming as default, do you think there is
On Sun, Sep 29, 2024 at 12:34 PM vignesh C wrote:
>
> On Thu, 26 Sept 2024 at 11:07, shveta malik wrote:
> >
> > On Fri, Sep 20, 2024 at 9:36 AM vignesh C wrote:
> > >
> > > On Wed, 21 Aug 2024 at 11:54, vignesh C wrote:
> > > >
> >
On Wed, Sep 18, 2024 at 3:31 PM shveta malik wrote:
>
> On Wed, Sep 18, 2024 at 2:49 PM shveta malik wrote:
> >
> > > > Please find the attached v46 patch having changes for the above review
> > > > comments and your test review comments and Shveta's re
On Fri, Sep 20, 2024 at 9:36 AM vignesh C wrote:
>
> On Wed, 21 Aug 2024 at 11:54, vignesh C wrote:
> >
> > On Wed, 21 Aug 2024 at 08:33, Peter Smith wrote:
> > >
> > > Hi Vignesh, Here are my only review comments for the latest patch set.
> >
> > Thanks, these issues have been addressed in the
On Fri, Sep 20, 2024 at 8:40 AM Nisha Moond wrote:
>
> Thanks for the review.
> Here is the v14 patch-set fixing review comments in [1] and [2].
>
Thanks for the patches. I am reviewing patch001, it is WIP, but please
find initial set of comments:
1)
Please see these 2 errors:
postgres=# create
On Wed, Sep 25, 2024 at 11:01 PM Bertrand Drouvot
wrote:
>
> Hi,
>
> On Wed, Sep 25, 2024 at 11:23:17AM +0530, shveta malik wrote:
> > + OUT catchange_xip xid[]
> >
> > One question, what is xid datatype, is it too int8? Sorry, could not
> > find the correct
On Mon, Sep 23, 2024 at 7:57 AM Peter Smith wrote:
>
> My review comments for v8-0001
>
> ==
> contrib/pg_logicalinspect/pg_logicalinspect.c
>
> 1.
> +/*
> + * Lookup table for SnapBuildState.
> + */
> +
> +#define SNAPBUILD_STATE_INCR 1
> +
> +static const char *const SnapBuildStateDesc[] = {
On Fri, Sep 20, 2024 at 12:22 PM Bertrand Drouvot
wrote:
>
>
> Please find attached v8, that:
>
Thank You for the patch. In one of my tests, I noticed that I got
negative checksum:
postgres=# SELECT * FROM pg_get_logical_snapshot_meta('0/3481F20');
magic| checksum | version
On Tue, Sep 24, 2024 at 10:23 PM Bertrand Drouvot
wrote:
>
> Hi,
>
> On Tue, Sep 24, 2024 at 09:15:31AM +0530, shveta malik wrote:
> > On Fri, Sep 20, 2024 at 12:22 PM Bertrand Drouvot
> > wrote:
> > >
> > >
> > > Please find attached v8, that:
On Thu, Sep 26, 2024 at 2:57 PM shveta malik wrote:
>
> On Fri, Sep 20, 2024 at 8:40 AM Nisha Moond wrote:
> >
> > Thanks for the review.
> > Here is the v14 patch-set fixing review comments in [1] and [2].
> >
>
> Thanks for the patches. I am reviewing pa
On Fri, Sep 27, 2024 at 10:44 AM shveta malik wrote:
>
> > >
> > > Thanks for the review.
> > > Here is the v14 patch-set fixing review comments in [1] and [2].
> > >
> >
> > Thanks for the patches. I am reviewing patch001, it is WIP, but ple
On Thu, Sep 19, 2024 at 5:43 PM vignesh C wrote:
>
> >
>
> I was reviewing the CONFLICT RESOLVER (insert_exists='apply_remote')
> and found that one conflict remains unresolved in the following
> scenario:
Thanks for the review and testing.
> Pub:
> CREATE TABLE circles(c1 CIRCLE, c2 text, EXCLU
On Mon, Sep 16, 2024 at 3:31 PM Bharath Rupireddy
wrote:
>
> Hi,
>
>
> Please find the attached v46 patch having changes for the above review
> comments and your test review comments and Shveta's review comments.
>
Thanks for addressing comments.
Is there a reason that we don't support this inva
On Wed, Sep 18, 2024 at 12:21 PM shveta malik wrote:
>
> On Mon, Sep 16, 2024 at 3:31 PM Bharath Rupireddy
> wrote:
> >
> > Hi,
> >
> >
> > Please find the attached v46 patch having changes for the above review
> > comments and your test
On Wed, Sep 18, 2024 at 2:49 PM shveta malik wrote:
>
> > > Please find the attached v46 patch having changes for the above review
> > > comments and your test review comments and Shveta's review comments.
> > >
When the synced slot is marked as 'inactive
On Thu, Sep 19, 2024 at 12:02 PM Amit Kapila wrote:
>
> On Tue, Sep 17, 2024 at 9:08 AM shveta malik wrote:
> >
> > On Mon, Sep 16, 2024 at 4:04 PM Amit Kapila wrote:
> > >
> > > On Mon, Sep 16, 2024 at 2:55 PM shveta malik
> > > wrote:
> >
On Mon, Sep 30, 2024 at 2:55 PM Peter Smith wrote:
>
> On Mon, Sep 30, 2024 at 4:29 PM shveta malik wrote:
> >
> > On Mon, Sep 30, 2024 at 11:04 AM Peter Smith wrote:
> > >
> > > On Mon, Sep 30, 2024 at 2:27 PM shveta malik
> > > wrote:
> >
On Tue, Oct 1, 2024 at 9:48 AM shveta malik wrote:
>
I have started reviewing patch002, it is WIP, but please find initial
set of comments:
1.
ExecSimpleRelationInsert():
+ /* Check for conflict and return to caller for resolution if found */
+ if (resolver != CR_ER
On Tue, Oct 1, 2024 at 9:54 AM shveta malik wrote:
>
> On Tue, Oct 1, 2024 at 9:48 AM shveta malik wrote:
> >
>
> I have started reviewing patch002, it is WIP, but please find initial
> set of comments:
>
Please find second set of comments for patch002:
9)
can_create_fu
On Thu, Sep 19, 2024 at 12:17 AM Bertrand Drouvot
wrote:
>
> Hi,
>
> Thanks for the review!
>
Thanks for the patch.
Should we include in the document who can execute these functions and
the required access permissions, similar to how it's done for
pgwalinspect, pg_ls_logicalmapdir(), and other s
On Tue, Sep 17, 2024 at 12:44 PM Bertrand Drouvot
wrote:
>
> Hi,
>
> On Tue, Sep 17, 2024 at 10:18:35AM +0530, shveta malik wrote:
> > Thanks for addressing the comments. I have not started reviewing v4
> > yet, but here are few more comments on v3:
> >
> >
On Fri, Apr 11, 2025 at 1:03 PM Nisha Moond wrote:
>
>
> Patch "v5_aprch_3-0001" implements the above Approach 3.
>
> Thanks Hou-san for implementing approach-2 and providing the patch.
>
I find Approach 2 better than Approach1. Yet to review Approach3.
Please find my initial comments:
Approach
On Tue, Apr 22, 2025 at 12:36 PM Zhijie Hou (Fujitsu)
wrote:
>
> Hi,
>
> To fix this, I think we can allow the base snapshot to be built during fast
> forward decoding, as implemented in the patch 0001 (We already built base
> snapshot in fast-forward mode for logical message in logicalmsg_decode(
On Mon, Apr 28, 2025 at 2:27 PM Zhijie Hou (Fujitsu)
wrote:
>
> On Fri, Apr 18, 2025 at 12:29 PM Amit Kapila wrote:
> >
> > On Thu, Apr 17, 2025 at 6:14 PM Zhijie Hou (Fujitsu)
> >
> > >
> > > -
> > > Fix
> > > -
> > >
> > > I think we should keep the confirmed_flush even if the previous
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
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 (
On Thu, Apr 24, 2025 at 2:54 PM Nisha Moond wrote:
>
> Please find the updated patch for Approach 3, which implements the
> idea suggested by Swada-san above.
>
Thank You for the patch.
1)
CreateDecodingContext:
if (ctx->twophase && !slot->data.two_phase)
{
+ /*
+ * Do not allow two-phase
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.
> > +
>
> 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 Fri, Apr 25, 2025 at 5:53 PM Nisha Moond wrote:
>
> Please find the attached v8 patch with above comments addressed.
> This version includes the documentation updates suggested by
> Sawada-san at [1], and incorporates his feedback from [2] as well.
>
Thanks for the patches.
1)
Regarding docu
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_tim
On Mon, Apr 28, 2025 at 4:33 PM Nisha Moond wrote:
>
> Please find the v9 patch, addressing the above and all other comments as well.
>
Thanks for the patch.
1)
+ The default is false. This option cannot be set together with
+ two_phase when creating the slot. However, once
+
On Fri, May 2, 2025 at 12:57 PM Nisha Moond wrote:
>
>
> Please find the v11 patch addressing the above points and all other
> comments. I have also optimized the test by reducing the number of
> subscriptions and slots.
>
Thanks for the patch. Few comments:
1)
pg_upgrade/t/003_logical_slots.pl
On Wed, May 7, 2025 at 4:36 PM Nisha Moond wrote:
>
>
> Attached is the v13 patch with the above comments addressed.
>
> --
Thanks for the patch.
I think we can have the restriction mentioned under the 'two_phase'
section as well along with the 'failover' section in the CREATE
SUBSCRIPTION doc,
On Sat, May 10, 2025 at 4:59 PM Amit Kapila wrote:
>
> On Tue, May 6, 2025 at 4:52 PM Zhijie Hou (Fujitsu)
> wrote:
> >
> > On Mon, May 5, 2025 at 6:59 PM Amit Kapila wrote:
> > >
> > >
> > > Yes, this is possible. Here is my theory as to how it can happen in the
> > > current
> > > case. In the
Hi All,
It is a spin-off thread from earlier discussions at [1] and [2].
While analyzing the slot-sync BF failure as stated in [1], it was
observed that there are chances that confirmed_flush_lsn may move
backward depending on the feedback messages received from the
downstream system. It was susp
On Sat, May 3, 2025 at 7:27 PM vignesh C wrote:
>
> >
> > Thanks for the comments, the updated patch has the changes for the same.
>
Thanks for the patches. Please find few comments:
1)
patch004 commit msg:
- Drop published sequences are removed from pg_subscription_rel.
Drop -->Dropped
2)
c
On Mon, May 5, 2025 at 3:29 PM Nisha Moond wrote:
>
> Please find the v12 patch with above suggested changes.
>
Thanks for the patch. Few comments for doc changes:
1)
func.sgml - pg_create_logical_replication_slot:
+failover. The parameters twophase and
+failover cannot be enabl
On Fri, May 16, 2025 at 12:17 PM Amit Kapila wrote:
>
> On Fri, Apr 25, 2025 at 10:08 AM shveta malik wrote:
> >
> > On Thu, Apr 24, 2025 at 6:11 PM Zhijie Hou (Fujitsu)
> > wrote:
> >
> > > > Few comments for patch004:
> > > > Config.sgml
On Fri, May 16, 2025 at 11:15 AM Amit Kapila wrote:
>
> On Fri, Apr 25, 2025 at 4:06 PM shveta malik wrote:
> >
> > 2)
> > in wait_for_local_flush(), we have
> > should_stop_conflict_info_retention() before 'AllTablesyncsReady'
> > check. Should we
On Thu, May 15, 2025 at 6:02 PM Amit Kapila wrote:
>
> On Fri, Apr 25, 2025 at 4:06 PM shveta malik wrote:
> >
> > >
> > > Here is V30 patch set includes the following changes:
> > >
> >
> > Thank You for the patch, please find few concerns:
>
Thanks you for v31 patch-set. Please find few comments on patch001:
1)
wait_for_local_flush:
+ if (data->last_recv_time &&
+ TimestampDifferenceExceeds(data->flushpos_update_time,
+data->last_recv_time, WalWriterDelay))
+ {
+ XLogRecPtr writepos;
+ XLogRecPtr flushpos;
+ bool have_pending_tx
On Tue, May 20, 2025 at 8:35 AM Nisha Moond wrote:
>
> >
> > Thanks for the comments, these are handled in the attached v20250516
> > version patch.
> >
>
> Thanks for the patches. Here are my review comments -
>
> Patch-0004: src/backend/replication/logical/sequencesync.c
>
> The sequence count l
On Tue, May 20, 2025 at 8:38 AM shveta malik wrote:
>
> Please find few more comments:
>
> 1)
> ProcessStandbyPSRequestMessage:
> + /*
> + * This shouldn't happen because we don't support getting primary status
> + * message from standby.
> + */
> + if (Re
On Tue, May 20, 2025 at 10:24 AM Amit Kapila wrote:
>
> On Tue, May 20, 2025 at 8:38 AM shveta malik wrote:
> >
> > Please find few more comments:
> >
> > 1)
> > ProcessStandbyPSRequestMessage:
> > + /*
> > + * This shouldn't happen because we
On Tue, May 20, 2025 at 11:13 AM Peter Smith wrote:
>
> > Test-scenario:
> > --Created 250 sequences on both pub and sub.
> > --There were 10 sequences mismatched.
> > --Sequence replication worked as expected. Logs look better now:
> >
> > LOG: Logical replication sequence synchronization for su
Please find few more comments:
1)
ProcessStandbyPSRequestMessage:
+ /*
+ * This shouldn't happen because we don't support getting primary status
+ * message from standby.
+ */
+ if (RecoveryInProgress())
+ elog(ERROR, "the primary status is unavailable during recovery");
a) This error is not cle
On Sat, May 24, 2025 at 10:37 AM Amit Kapila wrote:
> If the
> > problem is that we're not able to create a slot on the standby at an
> > old enough LSN or XID position to permit its use with the
> > corresponding slot on the master, it should be reported that way.
> >
>
> That is the case, and w
On Mon, May 26, 2025 at 12:02 PM shveta malik wrote:
>
> Agree that log messages need improvement. Please find the patch
> attached for the same. I also intend to update the docs in this area
> for users to understand this feature better, and will work on that
> soon.
>
PFA
On Mon, May 26, 2025 at 12:46 PM Zhijie Hou (Fujitsu)
wrote:
>
> Attaching the V32 patch set which addressed comments in [1]~[5].
Thanks for the patch, I am still reviewing the patches, please find
few trivial comments for patch001:
1)
+ FullTransactionId last_phase_at; /* publisher transaction
401 - 472 of 472 matches
Mail list logo
