On Thu, Aug 28, 2025 at 6:00 PM Nisha Moond wrote:
>
> Test-04. pgbench on both side, and max_conflict_retention_duration was tuned
>
> Setup:
> ---
> Pub --> Sub
> - setup is same as Test-03(
On Sat, Aug 30, 2025 at 10:17 AM Nisha Moond wrote:
>
> On Fri, Aug 29, 2025 at 11:49 AM Zhijie Hou (Fujitsu)
> wrote:
> >
> > Here is the new version patch set which also addressed Shveta's comments[1].
> >
>
> Thanks for the patches here, I tested the v68
Hi,
I ran tests to compare the performance of logical synchronous
replication with parallel-apply against physical synchronous
replication.
Highlights
===
On pgHead:(current behavior)
- With synchronous physical replication set to remote_apply, the
Primary’s TPS drops by ~60% (≈2.5x
On Fri, Aug 29, 2025 at 11:49 AM Zhijie Hou (Fujitsu)
wrote:
>
> Here is the new version patch set which also addressed Shveta's comments[1].
>
Thanks for the patches here, I tested the v68-001 patch alone, please
find review comments -
1) If a sub is created with retain_dead_tuples=on but disab
Hi,
As per the test results in [1], the TPS drop observed on the
subscriber with update_deleted enabled was mainly because only a
single apply worker was handling the replication workload from
multiple concurrent publisher clients.
The following performance benchmarks were conducted to evaluate th
On Thu, Aug 28, 2025 at 7:54 AM Zhijie Hou (Fujitsu)
wrote:
>
> Hi,
>
> My colleague Nisha reported an issue to me off-list: dead tuples can't
> be removed when retain_dead_tuples is enabled for a subscription with no
> tables.
>
> This appears to stem from the inability to advance the non-remova
On Wed, Aug 20, 2025 at 12:12 PM Zhijie Hou (Fujitsu)
wrote:
>
> I agree. Here is V63 version which implements this approach.
>
Thank you Hou-san for the patches. Here are couple of comments:
1) Once retention is stopped for all subscriptions and
conflict_slot.xmin is reset to NULL, we are no lo
On Wed, Aug 13, 2025 at 4:17 PM Zhijie Hou (Fujitsu)
wrote:
>
> Here is the initial POC patch for this idea.
>
Thank you Hou-san for the patch.
I did some performance benchmarking for the patch and overall, the
results show substantial performance improvements.
Please find the details as follows
On Mon, Aug 11, 2025 at 2:40 PM Zhijie Hou (Fujitsu)
wrote:
>
> I agree. So, following the above points and some off-list discussions, I have
> revised the option to be a subscription option in the V60 version.
>
Thanks Hou-san for the patches.
I have tested the patches and are working as expecte
On Wed, Aug 6, 2025 at 2:28 PM vignesh C wrote:
>
>
> The attached v20250806 version patch has the changes for the same.
>
Thank You for the patches.
patch-0005: sequencesync.c
+ aclresult = pg_class_aclcheck(RelationGetRelid(sequence_rel), GetUserId(),
+ ACL_UPDATE);
+ if (aclresult != ACLCHEC
Hi All,
We conducted performance testing of a bi-directional logical
replication setup, focusing on the primary use case of the
update_deleted feature.
To simulate a realistic scenario, we used a high workload with limited
concurrent updates, and well-distributed writes among servers.
Used source
On Thu, Jul 17, 2025 at 4:44 PM shveta malik wrote:
>
> On Thu, Jul 17, 2025 at 9:56 AM Dilip Kumar wrote:
> >
> > On Fri, Jul 11, 2025 at 4:28 PM Amit Kapila wrote:
> > >
> > > On Thu, Jul 10, 2025 at 6:46 PM Masahiko Sawada
> > > wrote:
> > > >
> > > > On Wed, Jul 9, 2025 at 9:09 PM Amit Kap
On Wed, Jul 9, 2025 at 4:11 PM vignesh C wrote:
>
> The attached v20250709 version patch has the changes for the same.
>
Thanks for the patches.
In Patch-004: sequencesync.c : I think below function logic can be simplified.
+void
+ProcessSyncingSequencesForApply(void)
+{
+ bool started_tx = fal
On Wed, Jul 9, 2025 at 5:39 PM Amit Kapila wrote:
>
> On Tue, Jul 8, 2025 at 12:18 AM Masahiko Sawada wrote:
> >
> > On Mon, Jul 7, 2025 at 12:03 PM Zhijie Hou (Fujitsu)
> > wrote:
> >
> > I think these performance regressions occur because at some point the
> > subscriber can no longer keep up
On Wed, Jun 25, 2025 at 3:10 PM Shlok Kyal wrote:
>
>
> 4. Since we are not adding sequences in the list 'sub_remove_rels',
> should we only palloc for (the count of no. of tables)? Is it worth
> the effort?
> /*
> * Rels that we want to remove from subscription and drop any slots
> * and origins
On Wed, Jun 25, 2025 at 9:26 AM shveta malik wrote:
>
> On Tue, Jun 24, 2025 at 6:44 PM Shlok Kyal wrote:
> >
> >
> > 1. Initially, I have created a publication on sequence s1.
> > postgres=# CREATE PUBLICATION pub1 FOR ALL SEQUENCES;
> > CREATE PUBLICATION
> > postgres=# ALTER PUBLICATION pub1 S
On Fri, Jun 27, 2025 at 8:50 AM shveta malik wrote:
>
> On Wed, Jun 25, 2025 at 7:42 PM Shlok Kyal wrote:
> > Hi Vignesh,
> >
> > I tested with all patches applied. I have a comment:
> >
> > Let consider following case:
> > On publisher create a publication pub1 on all sequence. publication
> > h
On Tue, Jun 24, 2025 at 3:07 PM Nisha Moond wrote:
>
> On Sun, Jun 22, 2025 at 8:05 AM vignesh C wrote:
> >
> > Thanks for the comment, the attached v20250622 version patch has the
> > changes for the same.
> >
>
> Thanks for the patches, please find my review
On Sun, Jun 22, 2025 at 8:05 AM vignesh C wrote:
>
> Thanks for the comment, the attached v20250622 version patch has the
> changes for the same.
>
Thanks for the patches, please find my review comments for patches 001 and 002:
1) patch-001 :pg_sequence_state()
+ /* open and lock sequence */
+
Hi,
Here are my review comments for v20250610 patches:
Patch-0005:sequencesync.c
1) report_error_sequences()
In case there are both missing and mismatched sequences, the ERROR
message logged is -
```
2025-05-28 14:22:19.898 IST [392259] ERROR: logical replication
sequence synchronization fail
On Thu, Jun 12, 2025 at 4:00 AM Peter Smith wrote:
>
> On Wed, Jun 11, 2025 at 8:16 PM Ajin Cherian wrote:
> >
> > On Fri, Jun 6, 2025 at 5:07 PM Nisha Moond wrote:
> > >
> > >
> > > Attached v18 patch.
> > > - patch-001: modified error mes
On Tue, Jun 10, 2025 at 2:29 PM shveta malik wrote:
>
> On Fri, Jun 6, 2025 at 12:37 PM Nisha Moond wrote:
> >
> > Attached v18 patch.
> > - patch-001: modified error messages as suggested above.
> > - patch-002: improved pg_dump docs as per Shveta's off-l
On Thu, Jun 5, 2025 at 3:26 PM Dilip Kumar wrote:
>
> On Thu, Jun 5, 2025 at 2:53 PM Dilip Kumar wrote:
> >
> > On Tue, Jun 3, 2025 at 11:05 AM Nisha Moond
> > wrote:
> > >
> > >
> > > Attached v17 patches. Added a top-up patch 0002
On Wed, Jun 4, 2025 at 11:16 PM Masahiko Sawada wrote:
>
> On Sun, Jun 1, 2025 at 10:25 PM Amit Kapila wrote:
> >
> > The one more combination to consider is when someone takes a dump of
> > an older version and loads it into a newer version. For example, where
> > users dump from 17.5 and then r
On Mon, Jun 2, 2025 at 10:55 AM Amit Kapila wrote:
>
> On Fri, May 30, 2025 at 3:00 PM Nisha Moond wrote:
> >
> > Agree that we need to cover the simple pg_dump and pg_restore with the
> > patch.
> >
> > When pg_dump and pg_restore are used outside of pg_up
On Fri, May 30, 2025 at 12:55 AM Masahiko Sawada wrote:
>
> On Thu, May 29, 2025 at 2:00 AM Nisha Moond wrote:
> >
> > On Wed, May 28, 2025 at 6:10 AM Masahiko Sawada
> > wrote:
> > >
> > > On Sun, May 25, 2025 at 11:34 PM Nisha Moond
> > >
On Wed, May 28, 2025 at 6:10 AM Masahiko Sawada wrote:
>
> On Sun, May 25, 2025 at 11:34 PM Nisha Moond wrote:
> >
> > to
> > On Fri, May 23, 2025 at 10:06 PM Masahiko Sawada
> > wrote:
> > >
> > >
> > > Here are review comme
On Thu, May 22, 2025 at 10:42 PM vignesh C wrote:
>
>
> The attached v20250522 patch has the changes for the same.
>
Thank you for the patches, please find comments for patch-0004.
1)
+/*
+ * report_error_sequences
+ *
+ * Logs a warning listing all sequences that are missing on the publisher,
+
to
On Fri, May 23, 2025 at 10:06 PM Masahiko Sawada wrote:
>
>
> Here are review comments for v14 patch:
>
Thank you for the review.
> I think we need to include a basic test case where we simply create a
> subscription with two_phase=true and then enable the failover via
> ALTER SUBSCRIPTION a
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:
> >
> > On Sun, May 4, 2025 at 2:33 PM Masahiko Sawada
> > wrote:
> > >
> > > While I cannot be entirely certain of my analysis, I believe the root
> > > cause might be related to the
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/logica
>
> 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 logic using curr_seq in copy_sequences() seems buggy.
Currently, curr_se
Hi,
On Tue, May 13, 2025 at 3:48 PM shveta malik wrote:
>
>
> With the given script, the problem reproduces on Head and PG17. We are
> trying to reproduce the issue on PG16 and below where injection points
> are not there.
>
The issue can also be reproduced on PostgreSQL versions 13 through 16.
On Sat, May 3, 2025 at 7:28 PM vignesh C wrote:
>
>
> There was one pending open comment #6 from [1]. This has been
> addressed in the attached patch.
Thank you for the patches, here are my comments for patch-004: sequencesync.c
copy_sequences()
---
1)
+ if (first)
+ first = fals
On Thu, May 8, 2025 at 11:35 AM shveta malik wrote:
>
> 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 hav
On Wed, May 7, 2025 at 10:32 AM shveta malik wrote:
>
> 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.sg
On Mon, May 5, 2025 at 11:39 AM David G. Johnston
wrote:
>
> On Sun, May 4, 2025 at 8:45 PM Nisha Moond wrote:
>>
>> Attached is the patch implementing the above proposed solution.
>> Reviews and feedback are most welcome.
>
>
> I feel like this is just paperi
On Fri, May 2, 2025 at 3:05 PM shveta malik wrote:
>
> 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
l correctly clean up what the tool created if something else goes
wrong later in the process.
Attached is the patch implementing the above proposed solution.
Reviews and feedback are most welcome.
--
Thanks,
Nisha
From ec934502f2da4e7822b851c040f9b832dd90b0a5 Mon Sep 17 00:00:00 2001
From: Nisha Moon
On Wed, Apr 30, 2025 at 5:45 AM Masahiko Sawada wrote:
>
> On Tue, Apr 29, 2025 at 5:00 AM Nisha Moond wrote:
>
> Thank you for updating the patch! Here are some comments on v10.
>
Thanks for reviewing the patch!
> ---
> +
> +# Also wait for two-phase to be
On Tue, Apr 29, 2025 at 2:51 PM shveta malik wrote:
>
> 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)
>
> +
On Mon, Apr 28, 2025 at 12:04 PM shveta malik wrote:
>
> 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-sa
On Thu, Apr 24, 2025 at 3:57 PM shveta malik wrote:
>
> 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.
>
&
On Thu, Apr 24, 2025 at 12:28 PM Amit Kapila wrote:
>
> On Wed, Apr 23, 2025 at 11:04 PM Masahiko Sawada
> wrote:
> >
> > On Tue, Apr 22, 2025 at 3:00 AM Amit Kapila wrote:
> > >
> > > On Mon, Apr 21, 2025 at 8:44 AM Zhijie Hou (Fujitsu)
> > > wrote:
> > > >
> > > > On Sat, Apr 19, 2025 at 2:1
On Tue, Apr 22, 2025 at 3:23 PM shveta malik wrote:
>
> 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 th
On Mon, Apr 21, 2025 at 9:26 PM Nathan Bossart wrote:
>
> Apparently replication origins in tests are supposed to be prefixed with
> "regress_". Here is a new patch with that fixed.
>
Hi Nathan,
Thanks for the patch! I tested it for functionality and here are a few comments:
1) While testing, I
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
HI,
Please find the patches attached for all three approaches.
On Wed, Apr 9, 2025 at 10:45 AM Zhijie Hou (Fujitsu)
wrote:
>
> On Thu, Apr 3, 2025 at 3:16 PM Zhijie Hou (Fujitsu) wrote:
> >
> > On Thu, Apr 3, 2025 at 1:38 PM Masahiko Sawada wrote:
> >
> > >
> > > On Wed, Apr 2, 2025 at 7:58 PM A
On Thu, Mar 20, 2025 at 5:23 PM Zhijie Hou (Fujitsu)
wrote:
>
> On Thu, Mar 20, 2025 at 3:06 PM Nisha Moond wrote:
> >
> > Attached is v6 patch with above comments addressed.
>
> Thanks updating the patch. I have some comments:
>
> 1.
>
> The naming style o
On Fri, Mar 21, 2025 at 3:38 PM Amit Kapila wrote:
>
> On Fri, Mar 21, 2025 at 1:48 PM Zhijie Hou (Fujitsu)
> wrote:
> >
> > On Fri, Mar 21, 2025 at 12:50 PM Nisha Moond wrote:
> >
> > > Thanks, Hou-san, for the review and fix patches. I’ve incorporated
> &
On Wed, Mar 19, 2025 at 4:18 PM Amit Kapila wrote:
>
> On Wed, Mar 19, 2025 at 11:11 AM Nisha Moond wrote:
> >
> > Please find the attached v5-0001 patch without the stats part.
> >
>
> Review:
> ===
> 1.
> + foreach_ptr(TupleTableSlot, slot, conflic
On Mon, Mar 17, 2025 at 3:20 PM Amit Kapila wrote:
>
> On Thu, Mar 13, 2025 at 4:30 PM Nisha Moond wrote:
> >
> > Attached is the v4 patch (test case changes only).
> >
>
> Comments:
> =
> 1.
> + /*
> + * Report an INSERT_EXISTS or U
Hi Shubham,
Here are a few comments for the v12 patch.
doc/src/sgml/ref/pg_createsubscriber.sgml :
1.
+
+ For all source server non-template databases create subscriptions for
+ create subscriptions for databases with the same names on the
+ target server.
is “create sub
false positives.
Note: I have broken down the full error detail message check into
multiple checks to avoid very long lines in the file. I'll see if
there's a better way to compare the full error detail in a single
check.
Attached is the v4 patch (test case changes only).
--
Thanks,
N
how that pans out.
> Meanwhile, I had a couple more replies below.
>
> On Tue, Feb 25, 2025 at 8:37 PM Nisha Moond wrote:
> >
> > On Mon, Feb 24, 2025 at 7:39 AM Peter Smith wrote:
> > >
> > > Hi Nisha.
> > >
> > > Some review comments for pat
On Mon, Mar 10, 2025 at 4:31 PM Shubham Khanna
wrote:
>
> > 2) This part of code has another bug:
> > "dbinfos.dbinfo->made_publication = false;" incorrectly modifies
> > dbinfo[0], even if the failure occurs in other databases (dbinfo[1],
> > etc.). Since cleanup_objects_atexit() checks made_pub
On Tue, Mar 11, 2025 at 11:10 AM Dilip Kumar wrote:
>
> On Thu, Feb 20, 2025 at 5:01 PM Nisha Moond wrote:
> >
> > Hi Hackers,
> > (CCing people involved in related discussions)
> >
> > I am starting this thread to propose a new conflict detection type
On Mon, Mar 10, 2025 at 12:11 PM Shubham Khanna
wrote:
>
> On Thu, Mar 6, 2025 at 9:27 AM Peter Smith wrote:
> >
> > 6.
> > - dbinfo->pubname, dbinfo->dbname, PQresultErrorMessage(res));
> > - dbinfo->made_publication = false; /* don't try again. */
> > + pubname, dbname, PQresultErrorMessage(res
On Tue, Mar 4, 2025 at 8:05 PM Shubham Khanna
wrote:
>
> The attached Patch contains the suggested changes.
>
Hi Shubham,
Here are few comments for 040_pg_createsubscriber.pl
1)
+# Run pg_createsubscriber on node S using '--cleanup-existing-publications'.
+# --verbose is used twice to show more
On Tue, Mar 4, 2025 at 10:30 AM Shubham Khanna
wrote:
>
> Fixed the suggested changes. The attached patch contains the required changes.
>
Hi Shubham,
Thanks for the patch! I tested its functionality and didn't find any
issues so far. I'll continue with further testing.
Here are some review comm
Hi,
Commit f41d846 [1] introduced a race condition that allows a process
to acquire and continue using an invalidated replication slot, leading
to unexpected behavior.
Example Scenario:
Suppose there is an orphaned logical slot that will be invalidated on
the next checkpoint. Consider the followi
On Mon, Feb 24, 2025 at 7:39 AM Peter Smith wrote:
>
> Hi Nisha.
>
> Some review comments for patch v1-0001
>
> ==
> GENERAL
>
> 1.
> This may be a basic/naive question, but it is unclear to me why we
> care about the stats of confl_multiple_unique_conflicts?
>
> I can understand it would be u
Hi Hackers,
(CCing people involved in related discussions)
I am starting this thread to propose a new conflict detection type
"multiple_unique_conflicts" that identifies when an incoming row
during logical replication violates more than one UNIQUE constraint.
If multiple constraints (such as the p
On Mon, Feb 17, 2025 at 11:29 AM Amit Kapila wrote:
>
> On Fri, Feb 14, 2025 at 5:30 PM Nisha Moond wrote:
> >
> > Here is a summary of changes in v78:
> >
>
> A few minor comments:
> 1.
> Slots that appear idle due to a disrupted connection between
> +
Please find the updated v78 patches after a few off-list review rounds.
Here is a summary of changes in v78:
patch-001:
- Fixed bugs reported by Hou-san and Peter in [1] and [2].
- Fixed a race condition reported by Hou-san off-list, which could
lead to an assert failure.
This failure happens when
On Tue, Feb 11, 2025 at 11:42 AM Zhijie Hou (Fujitsu)
wrote:
>
> On Monday, February 10, 2025 8:03 PM Nisha Moond
> wrote:
> >
> > On Sat, Feb 8, 2025 at 12:28 PM Zhijie Hou (Fujitsu)
> >
> > wrote:
> > >
> >
> > > 3.
> &g
On Tue, Feb 11, 2025 at 8:49 AM Peter Smith wrote:
>
> Hi Nisha.
>
> Some review comments about v74-0001
>
> ==
> src/backend/replication/slot.c
>
> 1.
> /* Maximum number of invalidation causes */
> -#define RS_INVAL_MAX_CAUSES RS_INVAL_WAL_LEVEL
> -
> -StaticAssertDecl(lengthof(SlotInvalida
On Mon, Feb 10, 2025 at 6:12 PM vignesh C wrote:
>
> On Mon, 10 Feb 2025 at 17:33, Nisha Moond wrote:
> >
> > Here are the v73 patches incorporating the comments above and the
> > subsequent comments from [1].
> > - patch 002 is rebased on 001 with no new chang
On Sat, Feb 8, 2025 at 12:28 PM Zhijie Hou (Fujitsu)
wrote:
>
> On Friday, February 7, 2025 9:06 PM Nisha Moond
> wrote:
> >
> > Attached v72 patches, addressed the above comments as well as Vignesh's
> > comments in [2].
> > - There are no new changes in
On Fri, Feb 7, 2025 at 8:00 AM Peter Smith wrote:
>
> Hi Nisha,
>
> Some review comments for v71-0001.
>
> ==
> src/backend/replication/slot.c
>
> SlotInvalidationCauses[]
>
> 2.
> [RS_INVAL_WAL_REMOVED] = "wal_removed",
> [RS_INVAL_HORIZON] = "rows_removed",
> [RS_INVAL_WAL_LEVEL] = "wa
On Thu, Feb 6, 2025 at 10:17 AM Amit Kapila wrote:
>
> On Thu, Feb 6, 2025 at 8:02 AM Nisha Moond wrote:
> >
> > On Wed, Feb 5, 2025 at 2:42 PM Amit Kapila wrote:
> > >
> > > Would it address your concern if we write the actual idle duration
> > &g
On Wed, Feb 5, 2025 at 2:42 PM Amit Kapila wrote:
>
> On Wed, Feb 5, 2025 at 10:30 AM vignesh C wrote:
> >
> > On Tue, 4 Feb 2025 at 19:56, Nisha Moond wrote:
> > >
> > > Here is v69 patch set addressing above and Kuroda-san's comments in [1].
> >
On Wed, Feb 5, 2025 at 12:58 PM Peter Smith wrote:
>
> Hi Nisha,
>
> Some review comments for the patch v69-0002.
>
> ==
> .../t/044_invalidate_inactive_slots.pl
>
> 2.
> +if ($ENV{enable_injection_points} ne 'yes')
> +{
> + plan skip_all => 'Injection points not supported by this build';
> +}
On Wed, Feb 5, 2025 at 10:30 AM vignesh C wrote:
>
> On Tue, 4 Feb 2025 at 19:56, Nisha Moond wrote:
> >
> > Here is v69 patch set addressing above and Kuroda-san's comments in [1].
>
> 2) Here we have mentioned about invalidation happens only for a)
>
On Tue, Feb 4, 2025 at 4:42 PM vignesh C wrote:
>
> On Tue, 4 Feb 2025 at 15:58, Nisha Moond wrote:
> >
> > Here are the v68 patches, incorporating above as well as comments from [1].
> >
> Few comments:
> 1) Let's call Timest
On Tue, Feb 4, 2025 at 10:45 AM Amit Kapila wrote:
>
> On Mon, Feb 3, 2025 at 6:35 PM Shlok Kyal wrote:
> >
> > I reviewed the v66 patch. I have few comments:
> >
> > 1. I also feel the default value should be set to '0' as suggested by
> > Vignesh in 1st point of [1].
> >
>
> +1. This will ensur
On Tue, Feb 4, 2025 at 9:33 AM Zhijie Hou (Fujitsu)
wrote:
>
> On Monday, February 3, 2025 8:03 PM Nisha Moond
> wrote:
> >
> > Hi Hackers,
> > (CC people involved in the earlier discussion)
> >
> > Right now, it is possible for the 'inactive_since&#x
from [1] and [2].
- No change in patch-003 since the last version.
[1]
https://www.postgresql.org/message-id/CALDaNm0FS%2BFqQk2dadiJFCMM_MhKROMsJUb%3Db8wtRH6isScQsQ%40mail.gmail.com
[2]
https://www.postgresql.org/message-id/CAHut%2BPs_6%2BNBOt%2BKpQQaBG2R3T-FLS93TbUC27uzyDMu%3D37n-Q%40mail.gmail.com
--
Thanks,
Ni
On Mon, Feb 3, 2025 at 2:55 PM Amit Kapila wrote:
>
> On Fri, Jan 31, 2025 at 5:50 PM Nisha Moond wrote:
> >
> > Please find the attached v66 patch set. The base patch(v65-001) is
> > committed now, so I have rebased the patches.
> >
>
> *
>
&
Hi Hackers,
(CC people involved in the earlier discussion)
Right now, it is possible for the 'inactive_since' value of an invalid
replication slot to be updated multiple times, which is unexpected
behavior.
As suggested in the ongoing thread [1], this patch introduces a new
dedicated function to u
On Fri, Jan 31, 2025 at 2:32 PM Amit Kapila wrote:
>
> On Fri, Jan 31, 2025 at 10:40 AM Peter Smith wrote:
> >
> > ==
> > src/backend/replication/slot.c
> >
> > ReportSlotInvalidation:
> >
> > 1.
> > +
> > + case RS_INVAL_IDLE_TIMEOUT:
> > + Assert(inactive_since > 0);
> > + /* translator: se
On Tue, Jan 28, 2025 at 5:28 PM Nisha Moond wrote:
>
> On Tue, Jan 28, 2025 at 3:26 PM Amit Kapila wrote:
> >
> > On Mon, Dec 30, 2024 at 11:05 AM Peter Smith wrote:
> > >
> > > I think we are often too quick to throw out perfectly good tests.
> >
On Thu, Jan 30, 2025 at 9:56 AM Amit Kapila wrote:
>
> On Thu, Jan 30, 2025 at 5:23 AM Peter Smith wrote:
> >
> > My understanding was that the purpose of this patch was not anything
> > to do with "optimisations" per se, but rather it was (like the
> > $SUBJECT says) to ensure the *same* 'active
On Tue, Jan 28, 2025 at 3:26 PM Amit Kapila wrote:
>
> On Mon, Dec 30, 2024 at 11:05 AM Peter Smith wrote:
> >
> > I think we are often too quick to throw out perfectly good tests.
> > Citing that some similar GUCs don't do testing as a reason to skip
> > them just seems to me like an example of
On Mon, Jan 27, 2025 at 4:20 PM Amit Kapila wrote:
>
> On Mon, Jan 27, 2025 at 11:00 AM Nisha Moond wrote:
> >
> > I discussed the above comments further with Peter off-list, and here
> > are the v63 patches with the following changes:
> > patch-001: The Assert
Hello Hackers,
(CC people involved in the earlier discussion)
While implementing slot invalidation based on inactive(idle) timeout
(see [1]), several general optimizations and improvements were
identified.
This thread is a spin-off from [1], intended to address these
optimizations separately from
On Wed, Jan 22, 2025 at 10:49 AM Nisha Moond wrote:
>
> On Tue, Jan 21, 2025 at 8:22 AM Peter Smith wrote:
> >
> > Some review comments for patch v61-0001.
> >
> > ==
> > src/backend/replication/slot.c
> >
> > 2.
> > + /*
> > + * T
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 Tue, Jan 21, 2025 at 8:22 AM Peter Smith wrote:
>
> Some review comments for patch v61-0001.
>
> ==
> src/backend/replication/slot.c
>
> InvalidatePossiblyObsoleteSlot:
>
> 1.
> /*
> * Check if the slot needs to be invalidated. If it needs to be
> - * invalidated, and is not currently a
On Tue, Jan 21, 2025 at 8:26 AM Peter Smith wrote:
>
> Some review comments for patch v61-0002
>
> ==
> src/backend/replication/slot.c
>
> 1.
> * Whether a slot needs to be invalidated depends on the cause. A slot is
> - * removed if it:
> + * invalidated if it:
> * - RS_INVAL_WAL_REMOVED:
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:
> > > > He
On Fri, Jan 17, 2025 at 6:50 PM Shlok Kyal wrote:
>
> Hi Nisha,
>
> Thanks for providing an updated patch. I have tested the patch and ran
> some tests. The patch works fine. I have few comments:
>
Thanks for your review. Attached are the v61 patches.
I've addressed the comments and rebased patc
On Mon, Jan 13, 2025 at 12:22 PM vignesh C wrote:
>
> On Thu, 2 Jan 2025 at 15:57, Nisha Moond wrote:
> >
> > Thank you for your feedback! Please find the v59 patch set addressing
> > all the comments.
> > Note: There are no new changes in patch-0001.
>
> Few
On Wed, Jan 15, 2025 at 11:37 AM Shlok Kyal wrote:
>
> On Thu, 2 Jan 2025 at 15:57, Nisha Moond wrote:
> >
> > On Thu, Jan 2, 2025 at 8:16 AM Peter Smith wrote:
> > >
> > > Hi Nisha,
> > >
> > > Here are some minor review comments for pat
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 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
> > -
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 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
nks,
Nisha
From 8154e2baee6fcf348524899c1f8b643a1e3564fc Mon Sep 17 00:00:00 2001
From: Nisha Moond
Date: Mon, 18 Nov 2024 16:13:26 +0530
Subject: [PATCH v59 1/2] Enhance replication slot error handling, slot
invalidation, and inactive_since setting logic
In ReplicationSlotAcquire(), raise an error for inval
On Thu, Jan 2, 2025 at 5:44 AM Peter Smith wrote:
>
> Hi Nisha.
>
> My review comments for patch v58-0001.
>
> ==
> src/backend/replication/slot.c
>
> InvalidatePossiblyObsoleteSlot:
>
> 1.
> /*
> - * If the slot can be acquired, do so and mark it invalidated
> - * immediately. Otherwise we
1 - 100 of 173 matches
Mail list logo
