On Mon, Nov 14, 2022 8:08 PM Melih Mutlu <m.melihmutlu@gmail.com> wrote:
>
> Attached patch with updated version of this patch.
Thanks for your patch.
I tried to do a performance test for this patch, the result looks good to me.
(The steps are similar to what Melih shared [1].)
The time to synchronize about 1GB data in binary (the average of the middle
eight was taken):
HEAD: 16.854 s
Patched: 6.805 s
Besides, here are some comments.
1.
+# Binary enabled subscription should fail
+$node_subscriber_binary->wait_for_log("ERROR: insufficient data left in message");
Should it be changed to "ERROR: ( [A-Z0-9]+:)? ", like other subscription tests.
2.
+# Binary disabled subscription should succeed
+$node_publisher->wait_for_catchup('tap_sub');
If we want to wait for table synchronization to finish, should we call
wait_for_subscription_sync()?
3.
I also think it might be better to support copy binary only for publishers of
v16 or later. Do you plan to implement it in the patch?
[1] https://www.postgresql.org/message-id/CAGPVpCQEKDVKQPf6OFQ-9WiRYB1YRejm--YJTuwgzuvj1LEJ2A%40mail.gmail.com
Regards,
Shi yu