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
v1-0001-Simplify-worker-type-checks.patch
Description: Binary data