| From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
|---|---|
| To: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
| Cc: | Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, shveta malik <shvetamalik(at)gmail(dot)com> |
| Subject: | Re: Proposal: Conflict log history table for Logical Replication |
| Date: | 2026-05-30 18:21:10 |
| Message-ID: | CAA4eK1JpyVs-AeN_BRZWxNHoMShCXz8YCKWy6w+NK0rHiKnA8g@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Fri, May 29, 2026 at 3:07 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Sat, May 30, 2026 at 3:24 AM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> After putting more thought, I think instead of executing a three-step
> process i.e. inserting the pg_subscription tuple, creating the table
> with its dependency, and then going back to update the tuple with the
> new relation ID, it is much cleaner to do it linearly, i.e. we should
> create the conflict log table first to get its OID, insert the
> subscription tuple pre-populated with that ID, and then record the
> dependency. This achieves the exact same state in a single direct
> sequence without the redundant catalog update within the same command.
> I agree with that code we would have to keep the record dependency
> code in CreateSubscription and AlterSubscription functions, but after
> putting more thought I think in thoese function we are already
> recording subscription dependencies with other object so wouldn't it
> be more natural to add this depednecy as well at the same place?
>
It makes sense to me and anyway for serverid also we are creating
dependency after creation of subscription, so your solution looks good
to me. One minor suggestion related to this changes:
+ if (CONFLICTS_LOGGED_TO_TABLE(opts.conflictlogdest))
+ {
+ ObjectAddress clt;
+
+ ObjectAddressSet(clt, RelationRelationId, logrelid);
+ recordDependencyOn(&clt, &myself, DEPENDENCY_INTERNAL);
+ }
Let's name clt as cltaddr or cltobj to make it consistent with naming
at some other similar places in code. Change this at both places where
we use this code.
> Anyway I am ready to change that if we have strong opinion against
> this approach.
>
> Here is the updated patch and changes are
> 1. 0003 and 0004 are merged on 0001
> 2. Merged Amit's v41_amit_1.patch.txt to 0002
> 3. Fix the dependency order issue (i.e. create dependency after
> inserting subscription tuple) and merged in 0002
>
> Open Items:
> 1. Need to create toast table for CLT after testing with larger JSON row
> 2. Fixed review comments of Shveta on 0004 and 0005
> 3. Rebase Vignesh's patch of
> "v41-0007-Preserve-conflict-log-destination-and-subscripti" I think we
> can do that once we have concensus on whether to create conflict log
> table first or insert the subscription row first as based on this
> change we would have to rebase this patch again.
> 4. Once we rebase
> "v41-0007-Preserve-conflict-log-destination-and-subscripti" after
> dependency order consensus I would rebase doc patch and \dRs+ change
> patch of Vignesh.
>
I see that my second comment in email [1] and another comment in email
[2] are still not answered and are neither listed in open items.
[1] - https://www.postgresql.org/message-id/CAA4eK1%2BzdaLF7%3DAVKd8xNGTuvPvn8BYSxHfnLZd7whWZ%2Bv3B-Q%40mail.gmail.com
[2] - https://www.postgresql.org/message-id/CAA4eK1K6tVUmKY-yqKgTX00yrSVAdSZN4Ao761JEXdtQkAYT4g%40mail.gmail.com
--
With Regards,
Amit Kapila.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Bruce Momjian | 2026-05-30 18:57:15 | Re: First draft of PG 19 release notes |
| Previous Message | Bruce Momjian | 2026-05-30 18:18:35 | Re: should we have a fast-path planning for OLTP starjoins? |