Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication
От | Peter Smith |
---|---|
Тема | Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication |
Дата | |
Msg-id | CAHut+Ps3Du9JFmhecWY8+VFD11VLOkSmB36t_xWHHQJNMpdA-A@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication (Melih Mutlu <m.melihmutlu@gmail.com>) |
Ответы |
Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication
(Amit Kapila <amit.kapila16@gmail.com>)
Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication (Melih Mutlu <m.melihmutlu@gmail.com>) |
Список | pgsql-hackers |
Here are some comments for patch v22-0001. ====== 1. General -- naming conventions There is quite a lot of inconsistency with variable/parameter naming styles in this patch. I understand in most cases the names are copied unchanged from the original functions. Still, since this is a big refactor anyway, it can also be a good opportunity to clean up those inconsistencies instead of just propagating them to different places. IIUC, the usual reluctance to rename things because it would cause backpatch difficulties doesn't apply here (since everything is being refactored anyway). E.g. Consider using use snake_case names more consistently in the following places: ~ 1a. start_table_sync +static void +start_table_sync(XLogRecPtr *origin_startpos, char **myslotname) +{ + char *syncslotname = NULL; origin_startpos -> (no change) myslotname -> my_slot_name (But, is there a better name for this than calling it "my" slot name) syncslotname -> sync_slot_name ~ 1b. run_tablesync_worker +static void +run_tablesync_worker() +{ + char originname[NAMEDATALEN]; + XLogRecPtr origin_startpos = InvalidXLogRecPtr; + char *slotname = NULL; + WalRcvStreamOptions options; originname -> origin_name origin_startpos -> (no change) slotname -> slot_name ~ 1c. set_stream_options +void +set_stream_options(WalRcvStreamOptions *options, + char *slotname, + XLogRecPtr *origin_startpos) +{ + int server_version; options -> (no change) slotname -> slot_name origin_startpos -> (no change) server_version -> (no change) ~ 1d. run_apply_worker static void -start_apply(XLogRecPtr origin_startpos) +run_apply_worker() { - PG_TRY(); + char originname[NAMEDATALEN]; + XLogRecPtr origin_startpos = InvalidXLogRecPtr; + char *slotname = NULL; + WalRcvStreamOptions options; + RepOriginId originid; + TimeLineID startpointTLI; + char *err; + bool must_use_password; originname -> origin_name origin_startpos => (no change) slotname -> slot_name originid -> origin_id ====== src/backend/replication/logical/worker.c 2. SetupApplyOrSyncWorker -ApplyWorkerMain(Datum main_arg) +SetupApplyOrSyncWorker(int worker_slot) { - int worker_slot = DatumGetInt32(main_arg); - char originname[NAMEDATALEN]; - XLogRecPtr origin_startpos = InvalidXLogRecPtr; - char *myslotname = NULL; - WalRcvStreamOptions options; - int server_version; - - InitializingApplyWorker = true; - /* Attach to slot */ logicalrep_worker_attach(worker_slot); + Assert(am_tablesync_worker() || am_leader_apply_worker()); + Why is the Assert not the very first statement of this function? ====== Kind Regards, Peter Smith. Fujitsu Australia
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Nathan BossartДата:
Сообщение: Re: [PATCH] Add loongarch native checksum implementation.