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:
I can simply change the places you mentioned, that seems okay to me.
The reason why I did not change the namings in existing variables/functions is because I did (and still do) not get what's the naming conventions in those files. Is snake_case the convention for variables in those files (or in general)?
Why is the Assert not the very first statement of this function?
I would also prefer to assert in the very beginning but am_tablesync_worker and am_leader_apply_worker require MyLogicalRepWorker to be not NULL. And MyLogicalRepWorker is assigned in logicalrep_worker_attach. I can change this if you think there is a better way to check the worker type.
