On Mon, Sept 19, 2022 11:26 AM Wang, Wei/王 威 <wangw.fnst@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