Re: refactoring basebackup.c

On Tue, Sep 21, 2021 at 10:50 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

>
> + if (opt->compression == BACKUP_COMPRESSION_LZ4)
>
> else if
>
> + /* First of all write the frame header to destination buffer. */
> + Assert(CHUNK_SIZE >= LZ4F_HEADER_SIZE_MAX);
> + headerSize = LZ4F_compressBegin(mysink->ctx,
> + mysink->base.bbs_next->bbs_buffer,
> + CHUNK_SIZE,
> + prefs);
>
> I think this is wrong. I think you should be passing bbs_buffer_length
> instead of CHUNK_SIZE, and I think you can just delete CHUNK_SIZE. If
> you think otherwise, why?
>
> + * sink's bbs_buffer of length that can accomodate the compressed input
>
> Spelling.
>
> + * Make it next multiple of BLCKSZ since the buffer length is expected so.
>
> The buffer length is expected to be a multiple of BLCKSZ, so round up.
>
> + * If we are falling short of available bytes needed by
> + * LZ4F_compressUpdate() per the upper bound that is decided by
> + * LZ4F_compressBound(), send the archived contents to the next sink to
> + * process it further.
>
> If the number of available bytes has fallen below the value computed
> by LZ4F_compressBound(), ask the next sink to process the data so that
> we can empty the buffer.
>

Thanks for your comments, Robert.
Here is the patch addressing the comments, except the one regarding the
autoFlush flag setting.

Kindly have a look.

Regards,
Jeevan Ladhe