On Wed, Dec 21, 2022 at 11:02 AM houzj.f...@fujitsu.com <houzj.f...@fujitsu.com> wrote: > > > Attach the new patch set which also includes some > cosmetic comment changes. >
I noticed one problem with the recent change in the patch. + * The fileset state should become FS_SERIALIZE_DONE once we have waited + * for a lock in the FS_SERIALIZE_IN_PROGRESS state, so we get the state + * again and recheck it later. + */ + if (fileset_state == FS_SERIALIZE_IN_PROGRESS) + { + pa_lock_stream(MyParallelShared->xid, AccessShareLock); + pa_unlock_stream(MyParallelShared->xid, AccessShareLock); + + fileset_state = pa_get_fileset_state(); + Assert(fileset_state == FS_SERIALIZE_DONE); This is not always true because say due to deadlock, this lock is released by the leader worker, in that case, the file state will be still in progress. So, I think we need a change like the below: diff --git a/src/backend/replication/logical/applyparallelworker.c b/src/backend/replication/logical/applyparallelworker.c index 45faa74596..8076786f0d 100644 --- a/src/backend/replication/logical/applyparallelworker.c +++ b/src/backend/replication/logical/applyparallelworker.c @@ -686,8 +686,8 @@ pa_spooled_messages(void) * the leader had serialized all changes which can lead to undetected * deadlock. * - * The fileset state must be FS_SERIALIZE_DONE once the leader worker has - * finished serializing the changes. + * Note that the fileset state can be FS_SERIALIZE_DONE once the leader + * worker has finished serializing the changes. */ if (fileset_state == FS_SERIALIZE_IN_PROGRESS) { @@ -695,7 +695,6 @@ pa_spooled_messages(void) pa_unlock_stream(MyParallelShared->xid, AccessShareLock); fileset_state = pa_get_fileset_state(); - Assert(fileset_state == FS_SERIALIZE_DONE); -- With Regards, Amit Kapila.