| From: | Zhijie Hou <houzhijie22(at)gmail(dot)com> |
|---|---|
| To: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Cc: | "amit(dot)kapila16(at)gmail(dot)com" <amit(dot)kapila16(at)gmail(dot)com> |
| Subject: | Re: Avoid resetting subscription two_phase state to pending when already enabled |
| Date: | 2026-10-29 07:37:15 |
| Message-ID: | fa4d521f-0413-4bbf-bded-0b6e47258b95@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Thu, Oct 29, 2026 at 03:29 PM "Hayato Kuroda (Fujitsu)" wrote:
>> While reviewing another logical replication thread, I noticed an issue
>> with altering a subscription's two_phase option.
>
> Good catch.
>
>> I think we can fix this by not resetting subtwophasestate to PENDING
>> when two_phase is already in the ENABLED state, as shown in the attached
>> patch.
>
> +1 the fix, but I have a quick confirmation.
>
> Assuming that twophasestate is 'e' and the user runs ALTER SUBSCRIPTION SET
> (two_phase = on). In this case all attributes are retained as-is but update_tuple
> is set to true anyway - CatalogTupleUpdate() would update the tuple with the
> completely the same tuple.
>
> But I feel it's acceptable because 1) it rarely happens,
> 2) conflict_log_destination option has the same issue, and 3) fixing it may
> need wider changes, like setting update_tuple in the if-statement, thought?
Right. This is the same as all (or most) subscription options and even
other object options like publication options have the same behavior,
since we don't compare the new value with the old one in most cases. So
I think that's an independent topic.
--
Best Regards,
Zhijie Hou
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Hayato Kuroda (Fujitsu) | 2026-10-29 07:56:13 | RE: Avoid resetting subscription two_phase state to pending when already enabled |
| Previous Message | Chao Li | 2026-10-29 07:30:07 | Re: tablecmds: Deadlock caused by Attach Partition |