On Mon, Sept 19, 2022 11:26 AM Wang, Wei/王 威 <wangw.f...@fujitsu.com> wrote: > > > Improved as suggested. >
Thanks for updating the patch. Here are some comments on 0001 patch. 1. + case TRANS_LEADER_SERIALIZE: - oldctx = MemoryContextSwitchTo(ApplyContext); + /* + * Notify handle methods we're processing a remote in-progress + * transaction. + */ + in_streamed_transaction = true; - MyLogicalRepWorker->stream_fileset = palloc(sizeof(FileSet)); - FileSetInit(MyLogicalRepWorker->stream_fileset); + /* + * Since no parallel apply worker is used for the first stream + * start, serialize all the changes of the transaction. + * + * Start a transaction on stream start, this transaction will be It seems that the following comment can be removed after using switch case. + * Since no parallel apply worker is used for the first stream + * start, serialize all the changes of the transaction. 2. + switch (apply_action) + { + case TRANS_LEADER_SERIALIZE: + if (!in_streamed_transaction) + ereport(ERROR, + (errcode(ERRCODE_PROTOCOL_VIOLATION), + errmsg_internal("STREAM STOP message without STREAM START"))); In apply_handle_stream_stop(), I think we can move this check to the beginning of this function, to be consistent to other functions. 3. I think the some of the changes in 0005 patch can be merged to 0001 patch, 0005 patch can only contain the changes about new column 'apply_leader_pid'. 4. + * ParallelApplyWorkersList. After successfully, launching a new worker it's + * information is added to the ParallelApplyWorkersList. Once the worker Should `it's` be `its` ? Regards Shi yu