Re: refactoring basebackup.c

On Sat, Jan 29, 2022 at 1:20 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Fri, Jan 28, 2022 at 12:48 PM Jeevan Ladhe
> <jeevan(dot)ladhe(at)enterprisedb(dot)com> wrote:
> > I have attached the latest rebased version of the LZ4 server-side
> compression
> > patch on the recent commits. This patch also introduces the compression
> level
> > and adds a tap test.
>
> In view of this morning's commit of
> d45099425eb19e420433c9d81d354fe585f4dbd6 I think the threshold for
> committing this patch has gone up. We need to make it support
> decompression with LZ4 on the client side, as we now have for gzip.
>

Fair enough. Makes sense.

> - In the new test case you set decompress_flags but according to the
> documentation I have here, -m is for multiple files (and so should not
> be needed here) and -d is for decompression (which is what we want
> here). So I'm confused why this is like this.
>
>
'-d' is the default when we have a .lz4 extension, which is true in our
case,
hence elimininated that. About, '-m' introduction, without any option, or
even
after providing the explicit '-d' option, weirdly lz4 command was throwing
decompressed tar on the console, that's when in my lz4 man version I saw
these 2 lines and tried adding '-m' option, and it worked:

" It is considered bad practice to rely on implicit output in scripts.
because the script´s environment may change. Always use explicit
output in scripts. -c ensures that output will be stdout. Conversely,
providing a destination name, or using -m ensures that the output will
be either the specified name, or filename.lz4 respectively."

and

"Similarly, lz4 -m -d can decompress multiple *.lz4 files."

> This part seems clearly correct, so I have committed it.

Thanks for pushing it.

Regards,
Jeevan Ladhe