From: | Peter Smith <smithpb2250(at)gmail(dot)com> |
---|---|
To: | Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> |
Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Adding a LogicalRepWorker type field |
Date: | 2023-08-04 07:45:07 |
Message-ID: | CAHut+PvBKYhOROVSHvbkPDjwi6gNmdtufZPS4ddQwoPm+9gBoA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Thank you for the feedback received so far. Sorry, I have become busy
lately with other work.
IIUC there is a general +1 for the idea, but I still have some
juggling necessary to make the functions/macros of patch 0001
acceptable to everybody.
The difficulties arise from:
- no function overloading C
- ideally, we prefer the same names for everything (e.g. instead of
dual-set macros)
- but launcher code calls need to pass param (other code always uses
MyLogicalRepWorker)
- would be nice (although no mandatory) to not have to always pass
MyLogicalRepWorker as a param.
Anyway, I will work towards finding some compromise on this next week.
Currently, I feel the best choice is to go with what Bharath suggested
in the first place -- just always pass the parameter (including
passing MyLogicalRepWorker). I will think more about it...
------
Meanwhile, here are some replies to other feedback received:
Ashutosh -- "May be we should create a union of type specific members" [1].
Yes, I was wondering this myself, but I won't pursue this yet until
getting over the hurdle of finding acceptable functions/macros for
patch 0001. Hopefully, we can come back to this idea.
~~
Mellih -- "Isn't the apply worker type still decided by eliminating
the other choices even with the patch?".
AFAIK the only case of that now is the one-time check in the
logicalrep_worker_launch() function. IMO, that is quite a different
proposition to the HEAD macros that have to make that deduction
10,000s ot times in executing code. Actually, even the caller knows
exactly the kind of worker it wants to launch so we can pass the
LogicalRepWorkerType directly to logicalrep_worker_launch() and
eliminate even this one-time-check. I can code it that way in the next
patch version.
~~
Barath -- "-1 for these names starting with prefix TYPE_, in fact LR_
looked better."
Hmmm. I'm not sure what is best. Of the options below I prefer
"WORKER_TYPE_xxx", but I don't mind so long as there is a consensus.
LR_APPLY_WORKER
LR_PARALLEL_APPLY_WORKER
LR_TABLESYNC_WORKER
TYPE_APPLY_WORKERT
TYPE_PARALLEL_APPLY_WORKER
TYPE_TABLESYNC_WORKER
WORKER_TYPE_APPLY
WORKER_TYPE_PARALLEL_APPLY
WORKER_TYPE_TABLESYNC
APPLY_WORKER
PARALLEL_APPLY_WORKER
TABLESYNC_WORKER
APPLY
PARALLEL_APPLY
TABLESYNC
------
[1] Ashutosh - https://www.postgresql.org/message-id/CAExHW5uALiimrdpdO0vwuDivD99TW%2B_9vvfFsErVNzq1ehYV9Q%40mail.gmail.com
[2] Melih - https://www.postgresql.org/message-id/CAGPVpCRZ-zEOa2Lkvw%2BiTU3NhJzfbGwH1dU7Htreup--8e5nxg%40mail.gmail.com
[3] Bharath - https://www.postgresql.org/message-id/CALj2ACVro6oCsTg_gpYu%2BV_LPMSgk4wjmSPDk8d5thArWNRoRQ%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
From | Date | Subject | |
---|---|---|---|
Next Message | Laetitia Avrot | 2023-08-04 08:26:46 | Re: Adding a pg_servername() function |
Previous Message | Drouvot, Bertrand | 2023-08-04 07:40:22 | Re: Synchronizing slots from primary to standby |