On Fri, Jul 21, 2023 at 7:30 AM Peter Smith <smithpb2250@gmail.com> wrote: How about SetupLogRepWorker? The other thing I noticed is that we don't seem to be consistent in naming functions in these files. For example, shall we make all exposed functions follow camel case (like InitializeLogRepWorker) and static functions follow _ style (like run_apply_worker) or the other possibility is to use _ style for all functions except may be the entry functions like ApplyWorkerMain()? I don't know if there is already a pattern but if not then let's form it now, so that code looks consistent.
I agree that these files have inconsistencies in naming things.
Most of the time I can't really figure out which naming convention I should use. I try to name things by looking at other functions with similar responsibilities.
> 3. > extern void pa_xact_finish(ParallelApplyWorkerInfo *winfo, > XLogRecPtr remote_lsn); > +extern void set_stream_options(WalRcvStreamOptions *options, > + char *slotname, > + XLogRecPtr *origin_startpos); > + > +extern void start_apply(XLogRecPtr origin_startpos); > +extern void DisableSubscriptionAndExit(void); > +extern void StartLogRepWorker(int worker_slot); > > This placement (esp. with the missing whitespace) seems to be grouping > the set_stream_options with the other 'pa' externs, which are all > under the comment "/* Parallel apply worker setup and interactions > */". > > Putting all these up near the other "extern void > InitializeLogRepWorker(void)" might be less ambiguous. >
+1. Also, note that they should be in the same order as they are in .c files.
I did not realize the order is the same with .c files. Good to know. I'll fix it along with other comments.
