Re: refactoring basebackup.c - Mailing list pgsql-hackers

From Jeevan Ladhe
Subject Re: refactoring basebackup.c
Date
Msg-id CAOgcT0M+RYf8+3Wjx_YVfZD99ZvzMZX7anMR7XKXR6VobApMEQ@mail.gmail.com
Whole thread Raw
In response to Re: refactoring basebackup.c  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers

On Tue, Sep 21, 2021 at 10:50 PM Robert Haas <robertmhaas@gmail.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
 
Attachment

pgsql-hackers by date:

Previous
From: Jeevan Ladhe
Date:
Subject: Re: refactoring basebackup.c
Next
From: "Euler Taveira"
Date:
Subject: Re: logical replication restrictions