PSA v4 patches. On Fri, Aug 4, 2023 at 5:45 PM Peter Smith <smithpb2...@gmail.com> wrote: > > 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...
v4-0001 uses only 3 simple inline functions. Callers always pass parameters as Bharath had suggested. > > ------ > > 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. > To be explored later. > ~~ > > 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. > Now even the one-time checking to assign the worker type is removed. The callers know the LogicalReWorkerType they want, so v4-0001 just passes the type into logicalrep_worker_launch() > ~~ > > 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 > For now, in v4-0001 these are called WORKERTYPE_xxx. Please do propose better names if these are no good. ------ Kind Regards, Peter Smith. Fujitsu Australia
v4-0001-Add-LogicalRepWorkerType-enum.patch
Description: Binary data
v4-0002-Switch-on-worker-type.patch
Description: Binary data
