From: | Peter Smith <smithpb2250(at)gmail(dot)com> |
---|---|
To: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Simplify some logical replication worker type checking |
Date: | 2023-08-01 01:35:35 |
Message-ID: | CAHut+Pv-xkEpuPzbEJ=ZSi7Hp2RoGJf=VA-uDRxLi1KHSneFjg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi hackers,
BACKGROUND
There are 3 different types of logical replication workers.
1. apply leader workers
2. parallel apply workers
3. tablesync workers
And there are a number of places where the current worker type is
checked using the am_<worker-type> inline functions.
PROBLEM / SOLUTION
During recent reviews, I noticed some of these conditions are a bit unusual.
======
worker.c
1. apply_worker_exit
/*
* Reset the last-start time for this apply worker so that the launcher
* will restart it without waiting for wal_retrieve_retry_interval if the
* subscription is still active, and so that we won't leak that hash table
* entry if it isn't.
*/
if (!am_tablesync_worker())
ApplyLauncherForgetWorkerStartTime(MyLogicalRepWorker->subid);
~
In this case, it cannot be a parallel apply worker (there is a check
prior), so IMO it is better to simplify the condition here to below.
This also makes the code consistent with all the subsequent
suggestions in this post.
if (am_apply_leader_worker())
ApplyLauncherForgetWorkerStartTime(MyLogicalRepWorker->subid);
~~~
2. maybe_reread_subscription
/* Ensure we remove no-longer-useful entry for worker's start time */
if (!am_tablesync_worker() && !am_parallel_apply_worker())
ApplyLauncherForgetWorkerStartTime(MyLogicalRepWorker->subid);
proc_exit(0);
~
Should simplify the above condition to say:
if (!am_leader_apply_worker())
~~~
3. InitializeApplyWorker
/* Ensure we remove no-longer-useful entry for worker's start time */
if (!am_tablesync_worker() && !am_parallel_apply_worker())
ApplyLauncherForgetWorkerStartTime(MyLogicalRepWorker->subid);
proc_exit(0);
~
Ditto. Should simplify the above condition to say:
if (!am_leader_apply_worker())
~~~
4. DisableSubscriptionAndExit
/* Ensure we remove no-longer-useful entry for worker's start time */
if (!am_tablesync_worker() && !am_parallel_apply_worker())
ApplyLauncherForgetWorkerStartTime(MyLogicalRepWorker->subid);
~
Ditto. Should simplify the above condition to say:
if (!am_leader_apply_worker())
------
PSA a small patch making those above-suggested changes. The 'make
check' and TAP subscription tests are all passing OK.
-------
Kind Regards,
Peter Smith.
Fujitsu Australia
Attachment | Content-Type | Size |
---|---|---|
v1-0001-Simplify-worker-type-checks.patch | application/octet-stream | 1.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2023-08-01 02:00:32 | Re: Ignore 2PC transaction GIDs in query jumbling |
Previous Message | Julien Rouhaud | 2023-08-01 01:28:08 | Re: Ignore 2PC transaction GIDs in query jumbling |