Re: Allow logical replication to copy tables in binary format

From: Melih Mutlu <m(dot)melihmutlu(at)gmail(dot)com>
To: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, "Takamichi Osumi (Fujitsu)" <osumi(dot)takamichi(at)fujitsu(dot)com>, Jelte Fennema <postgres(at)jeltef(dot)nl>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, "shiy(dot)fnst(at)fujitsu(dot)com" <shiy(dot)fnst(at)fujitsu(dot)com>, Euler Taveira <euler(at)eulerto(dot)com>
Subject: Re: Allow logical replication to copy tables in binary format
Date: 2023-02-27 19:52:29
Message-ID: CAGPVpCTAKDraT9Y8OW3u0b+sKswDcbaruNN+RN+fVE0LAYP4qg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

Thanks for all of your reviews!

So, I made some changes in the v10 according to your comments.

1- copy_format is changed to a boolean parameter copy_binary.
It sounds easier to use a boolean to enable/disable binary copy. Default
value is false, so nothing changes in the current behaviour if copy_binary
is not specified.
It's still persisted into the catalog. This is needed since its value will
be needed by tablesync workers later. And tablesync workers fetch
subscription configurations from the catalog.
In the copy_data case, it is not directly stored anywhere but it affects
the state of the table which is stored in the catalog. So, I guess this is
the convenient way if we decide to go with a new parameter.

2- copy_binary is not tied to another parameter
The patch does not disallow any combination of copy_binary with binary and
copy_data options. I tried to explain what binary copy can and cannot do in
the docs. Rest is up to the user now.

3- Removed version check for copy_binary
HEAD already does not have any check for binary option. Making binary copy
work only if publisher and subscriber are the same major version can be too
restricted.

4- Added separate test file
Although I believe 002_types.pl and 014_binary.pl can be improved more for
binary enabled subscription cases, this patch might not be the best place
to make those changes.
032_binary_copy.pl now has the tests for binary copy option. There are also
some regression tests in subscription.sql.

Finally, some other small fixes are done according to the reviews.

Also, thanks Bharath for performance testing [1]. It can be seen that the
patch has some benefits.

I appreciate further thoughts/reviews.

[1]
https://www.postgresql.org/message-id/CALj2ACUfE08ZNjKK-nK9JiwGhwUMRLM%2BqRhNKTVM9HipFk7Fow%40mail.gmail.com

Thanks,
--
Melih Mutlu
Microsoft

Attachment Content-Type Size
v10-0001-Allow-logical-replication-to-copy-table-in-binar.patch application/octet-stream 65.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2023-02-27 19:58:30 Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
Previous Message Andres Freund 2023-02-27 19:50:27 Re: tests against running server occasionally fail, postgres_fdw & tenk1