On Wed, Jan 10, 2024 at 06:02:12PM +0530, Amit Kapila wrote: > - if (max_logical_replication_workers == 0) > + /* > + * The logical replication launcher is disabled during binary upgrades, > + * as logical replication workers can stream data during the upgrade > + * which can cause replication origins to move forward after we have > + * copied them. It can cause the system to request the data which is > + * already present in the new cluster. > + */ > + if (max_logical_replication_workers == 0 || IsBinaryUpgrade) > > This comment is not very clear to me. The first part of the sentence > can't apply to the new cluster as after the upgrade, subscriptions > will be disabled and the second part talks about requesting the wrong > data in the new cluster. As per my understanding, the problem here is > that, on the old cluster, the origins may move forward after we copy > them and then we copy physical files. Now, in the new cluster when we > try to request the data, it will be already present.
As far as I understand your complaint is about being more precise about where the workers could run when we do an upgrade. My patch covers the reason why it would be a problem, and I agree that it could be more detailed. Hence, how about something like that: "The logical replication launcher is disabled during binary upgrades, as a logical replication workers running on the cluster upgrading from could cause replication origins to move forward after they are copied to the cluster upgrading to, creating potentially conflicts with the physical files copied over." If you have a better suggestion, feel free. -- Michael
signature.asc
Description: PGP signature
