Date: Fri, 14 Jan 2022 12:23:27 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Georgios Kokolatos <gkokolatos@pm.me>,
	Postgres hackers <pgsql-hackers@lists.postgresql.org>,
	Jeevan Ladhe <jeevan.ladhe@enterprisedb.com>
Subject: Re: Refactoring of compression options in pg_basebackup
Message-ID: <YeDsr4rVko87dHWd@paquier.xyz>
References: <Yb3GEgWwcu4wZDuA@paquier.xyz>
 <YuCGAJCBv6dd06z-v1gysn6F69MBWxspGLLGa8wJBaj8BrYs_rxJT3BJuh1U_LEDghumIN2N9ed-Gfve7K2dBeseCCOsypharQkXknnrxeo=@pm.me>
 <YdVQICWlyMA2NBSN@paquier.xyz>
 <pAYKvZybUu5ercp9Supt5f0OVFKEJF24WGzdmoBo9ReJtBKs4zFILwcn4J1_6f4XjHfrWxFZlPa83qyh57jmelXTIc3LsONWbqXJIuJyg1Y=@pm.me>
 <CA+TgmoYb4jnOU+-Xipf2V+twF9MR1P9bbLQqjiAmi3yc3r1tOA@mail.gmail.com>
 <YdZ4R7sNek759eZX@paquier.xyz>
 <CA+TgmoaUW0v5oxaDTu05n_r--iy6DJFhsxQ2Ldbtf20ZkHm1HA@mail.gmail.com>
 <Ydfg+r7SNZ+HSJst@paquier.xyz>
 <CA+TgmoZ5+Tis+ow5R=GtwoFkMRvf0CqM=bDUXx+wQioND+coYg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="WPxTUyJ4dLUY5oHY"
Content-Disposition: inline
In-Reply-To: <CA+TgmoZ5+Tis+ow5R=GtwoFkMRvf0CqM=bDUXx+wQioND+coYg@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/YeDsr4rVko87dHWd%40paquier.xyz>
Precedence: bulk


--WPxTUyJ4dLUY5oHY
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Jan 13, 2022 at 01:36:00PM -0500, Robert Haas wrote:
> 1. If, as you propose, we add a new flag --compression-method=3DMETHOD
> then how will the user specify server-side compression?

This would require a completely different option switch, which is
basically the same thing as what you are suggesting with
--server-compress.

> 2. If, as we seem to agree, the compression method is more important
> than the compression level, then why is the option to set the
> less-important thing called just --compress, and the option to set the
> more important thing has a longer name?

I agree that the method is more important than the level for most
users, and I would not mind dropping completely --compress in favor of
something else, which is something I implied upthread.

> I proposed to solve both of these problems by using
> --compression-level=3DNUMBER to set the compression level and
> --compress=3DMETHOD or --server-compress=3DMETHOD to set the algorithm and
> specify on which side it is to be applied. If, instead of doing that,
> we go with what you have proposed here, then I don't understand how to
> fit server-side compression into the framework in a reasonably concise
> way. I think we would end up with something like pg_basebackup
> --compression-method=3Dlz4 --compress-on-server, which seems rather long
> and awkward. Do you have a better idea?

Using --compression-level=3DNUMBER and --server-compress=3DMETHOD to
specify a server-side compression method with a level is fine by me,
but I find the reuse of --compress to specify a compression method=20
confusing as it maps with the past option we have kept in
pg_basebackup for a couple of years now.  Based on your suggested set
of options, we could then have a --client-compress=3DMETHOD and
--compression-level=3DNUMBER to specify a client-side compression method
with a level.  If we do that, I guess that we should then:
1) Block the combination of --server-compress and --client-compress.
2) Remove the existing -Z/--compress and -z/--gzip.

You have implied 1) upthread as far as I recall, 2) is something I am
adding on top of it.

> I think I understand what the patch is doing. I just think it creates
> a problem for my patch. And I'd like to know whether you have an idea
> how to solve that problem. And if not, then I'd like you to consider
> the solution that I am proposing rather than the patch you've already
> got.

I am fine to drop this thread's patch with its set of options and work
on top of your proposal, aka what's drafted two paragraphs above.
--
Michael

--WPxTUyJ4dLUY5oHY
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmHg7K8ACgkQnvQgOdby
QH1DoQ/9EVkSmQ/jmIF/+O/NbtVXCcuI9Oi8Vyx4wn65lPirXCV9U9devCOCJgI0
5wigkIHGtqShSA9Kc74tRzSqzwCadSCPz5lSNfVoEqMYEOt1V4t3FtG/ZHOBfyZw
dXUjb00e/QIYl5eWr7D6I0TYJILtKTnB7LuynytaP0O1ucMfI7dERUI5B31d9bAj
Ge0OigwzUIPPOx1ceJqiRUbkRibMwcxI1XzV+jYo7PvDlHeNbV69gSi6GVn+6jn/
U0DW6ZcCbt1RcM/fHf2W/lzJjV81QTR4x4QM/w0SpsZVW3F7JiDKajHIiVibh/WZ
5KJSs7DHBolSs9q7JhhKnErXi/pmAbcqF+Z5UXmR6zXLpHpHJv42VApcvJt4W85E
O1vNswEqP0JHOKbyRVKLEVy0n/YkblP7dyWVM1k7dSLkxHwrL8M+0ywzMfNVk4wN
xFLFJ15wlCnSNC11WHfTyD8BOPaS2m+41wpQarUiK9tbl8rdMkwcg7zclDJ9mAiz
twfj/0dV5X5zdLX/uk/Hwco8o5rn/X7hOCc4IwSsqlO0UCJ+D5VWh+5qnIoivTTX
4fjfnYO3AOCWta55xk5ell8Z8foPYHZHXJn2qQ+FY3kU00tmm5cd6VEPrEhQp8qW
mqbVYjOWE4aRpI4E9XQfKh8NZlWpBd21QCiZt2n9gUYbHmvUCX4=
=iMBG
-----END PGP SIGNATURE-----

--WPxTUyJ4dLUY5oHY--