Refactoring of replication commands using printsimple

Started by Michael Paquierabout 9 years ago5 messageshackers
Jump to latest
#1Michael Paquier
michael@paquier.xyz

Hi all,

This is a follow up of the refactoring that has been discussed in the
thread to increase the default size of WAL segments:
/messages/by-id/CAB7nPqQ4hyNrLq+W1JrrYVSySOxuQa40PYb2Uw5UQkKaG4hong@mail.gmail.com

The discussion has resulted in the creation of a84069d9 that has
introduced a new DestReceiver method called printsimple that does not
need any catalog access. After some investigation, I have noticed that
a couple of messages used in the replication protocol could be
refactored as well:
- IDENTIFY_SYSTEM
- TIMELINE_HISTORY
- CREATE_REPLICATION_SLOT
This results in the following code reduction:
3 files changed, 115 insertions(+), 162 deletions(-)

A commit fest entry has been created:
https://commitfest.postgresql.org/13/978/

Thanks,
--
Michael

Attachments:

refactor-repl-cmd-output-v3.patchapplication/octet-stream; name=refactor-repl-cmd-output-v3.patchDownload+115-162
#2Robert Haas
robertmhaas@gmail.com
In reply to: Michael Paquier (#1)
Re: Refactoring of replication commands using printsimple

On Tue, Jan 31, 2017 at 12:19 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:

This is a follow up of the refactoring that has been discussed in the
thread to increase the default size of WAL segments:
/messages/by-id/CAB7nPqQ4hyNrLq+W1JrrYVSySOxuQa40PYb2Uw5UQkKaG4hong@mail.gmail.com

The discussion has resulted in the creation of a84069d9 that has
introduced a new DestReceiver method called printsimple that does not
need any catalog access. After some investigation, I have noticed that
a couple of messages used in the replication protocol could be
refactored as well:
- IDENTIFY_SYSTEM
- TIMELINE_HISTORY
- CREATE_REPLICATION_SLOT
This results in the following code reduction:
3 files changed, 115 insertions(+), 162 deletions(-)

A commit fest entry has been created:
https://commitfest.postgresql.org/13/978/

Sorry, I have a little more nitpicking. How about having
printsimple() use pq_sendcountedtext() instead of pq_sendint()
followed by pq_sendbytes(), as it does for TEXTOID?

Other than that, this looks fine to me now.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Michael Paquier
michael@paquier.xyz
In reply to: Robert Haas (#2)
Re: Refactoring of replication commands using printsimple

On Tue, Jan 31, 2017 at 11:59 PM, Robert Haas <robertmhaas@gmail.com> wrote:

Sorry, I have a little more nitpicking.

Thanks for the input.

How about having
printsimple() use pq_sendcountedtext() instead of pq_sendint()
followed by pq_sendbytes(), as it does for TEXTOID?

Other than that, this looks fine to me now.

pq_sendcountedtext() does some encoding conversion, which is why I
haven't used because we deal only with integers in this patch... Now
if you wish to switch to that I have really no arguments against.
--
Michael

Attachments:

refactor-repl-cmd-output-v4.patchapplication/octet-stream; name=refactor-repl-cmd-output-v4.patchDownload+114-162
#4Robert Haas
robertmhaas@gmail.com
In reply to: Michael Paquier (#3)
Re: Refactoring of replication commands using printsimple

On Tue, Jan 31, 2017 at 8:26 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

pq_sendcountedtext() does some encoding conversion, which is why I
haven't used because we deal only with integers in this patch... Now
if you wish to switch to that I have really no arguments against.

OK, I see. Well, I guess it's sensible either way, but I've committed
this version. Thanks.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Michael Paquier
michael@paquier.xyz
In reply to: Robert Haas (#4)
Re: Refactoring of replication commands using printsimple

On Thu, Feb 2, 2017 at 4:05 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Tue, Jan 31, 2017 at 8:26 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

pq_sendcountedtext() does some encoding conversion, which is why I
haven't used because we deal only with integers in this patch... Now
if you wish to switch to that I have really no arguments against.

OK, I see. Well, I guess it's sensible either way, but I've committed
this version. Thanks.

Thanks.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers