Some minor review comments for v58-0001 ======
.../replication/logical/applyparallelworker.c 1. pa_can_start + /* + * Don't start a new parallel worker if user has set skiplsn as it's + * possible that user want to skip the streaming transaction. For streaming + * transaction, we need to serialize the transaction to a file so that we + * can get the last LSN of the transaction to judge whether to skip before + * starting to apply the change. + */ + if (!XLogRecPtrIsInvalid(MySubscription->skiplsn)) + return false; "that user want" -> "that they want" "For streaming transaction," -> "For streaming transactions," ~~~ 2. pa_free_worker_info + /* Remove from the worker pool. */ + ParallelApplyWorkerPool = list_delete_ptr(ParallelApplyWorkerPool, + winfo); Unnecessary wrapping ~~~ 3. pa_set_stream_apply_worker +/* + * Set the worker that required to apply the current streaming transaction. + */ +void +pa_set_stream_apply_worker(ParallelApplyWorkerInfo *winfo) +{ + stream_apply_worker = winfo; +} Comment wording seems wrong. ====== src/include/replication/worker_internal.h 4. ParallelApplyWorkerShared + * XactLastCommitEnd from the parallel apply worker. This is required to + * update the lsn_mappings by leader worker. + */ + XLogRecPtr last_commit_end; +} ParallelApplyWorkerShared; "This is required to update the lsn_mappings by leader worker." --> did you mean "This is required by the leader worker so it can update the lsn_mappings." ?? ------ Kind Regards, Peter Smith. Fujitsu Australia