From: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
---|---|
To: | "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com> |
Cc: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: wake up logical workers after ALTER SUBSCRIPTION |
Date: | 2022-11-23 20:50:27 |
Message-ID: | 20221123205027.GA368966@nathanxps13 |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Nov 22, 2022 at 07:25:36AM +0000, houzj(dot)fnst(at)fujitsu(dot)com wrote:
> On Tuesday, November 22, 2022 2:49 PM Hayato Kuroda (Fujitsu) <kuroda(dot)hayato(at)fujitsu(dot)com>
>> Thanks for updating! It becomes better. Further comments:
>>
>> 01. AlterSubscription()
>>
>> ```
>> + LogicalRepWorkersWakeupAtCommit(subid);
>> +
>> ```
>>
>> Currently subids will be recorded even if the subscription is not modified.
>> I think LogicalRepWorkersWakeupAtCommit() should be called inside the if
>> (update_tuple).
>
> I think an exception would be REFRESH PULLICATION in which case update_tuple is
> false, but it seems better to wake up apply worker in this case as well,
> because the apply worker is also responsible to start table sync workers for
> newly subscribed tables(in process_syncing_tables()).
>
> Besides, it seems not a must to wake up apply worker for ALTER SKIP TRANSACTION,
> Although there might be no harm for waking up in this case.
In v3, I moved the call to LogicalRepWorkersWakeupAtCommit() to the end of
the function. This should avoid waking up workers in some cases where it's
unnecessary (e.g., if ALTER SUBSCRIPTION ERRORs in a subtransaction), but
there are still cases where we'll wake up the workers unnecessarily. I
think this is unlikely to cause any real problems in practice.
>> 02. LogicalRepWorkersWakeupAtCommit()
>>
>> ```
>> + oldcxt = MemoryContextSwitchTo(TopTransactionContext);
>> + on_commit_wakeup_workers_subids =
>> lappend_oid(on_commit_wakeup_workers_subids,
>> +
>> subid);
>> ```
>>
>> If the subscription is altered twice in the same transaction, the same subid will
>> be recorded twice.
>> I'm not sure whether it may be caused some issued, but list_member_oid() can
>> be used to avoid that.
>
> +1, list_append_unique_oid might be better.
Done in v3.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachment | Content-Type | Size |
---|---|---|
v3-0001-wake-up-logical-workers-after-ALTER-SUBSCRIPTION.patch | text/x-diff | 4.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Justin Pryzby | 2022-11-23 20:52:22 | Re: Add LZ4 compression in pg_dump |
Previous Message | Tom Lane | 2022-11-23 20:48:04 | Re: drop postmaster symlink |