On Thu, Nov 04, 2021 at 05:02:28PM +0000, gkokolatos@pm.me wrote:
> Removed an extra condinional check while switching over compression_method.
Indeed. My rebase was a bit sloppy here.
> because compression_method is the global option exposed to the whereas
> wal_compression_method is the local variable used to figure out what kind of
> file the function is currently working with. This error was existing at least in
> v9-0002 of $subject.
Right.
> I felt that converting it a do {} while () loop instead, will help with
> readability:
> + do
> + {
> <snip>
> + /*
> + * No need to continue reading the file when the uncompressed_size
> + * exceeds WalSegSz, even if there are still data left to read.
> + * However, if uncompressed_size is equal to WalSegSz, it should
> + * verify that there is no more data to read.
> + */
> + } while (r > 0 && uncompressed_size <= WalSegSz);
No objections from me to do that. This makes the code a bit easier to
follow, indeed.
> I would like to have a bit more test coverage in the case for FindStreamingStart().
> Specifically for the case that a lz4-compressed segment larger than WalSegSz exists.
The same could be said for gzip. I am not sure that this is worth the
extra I/O and pg_receivewal commands, though.
I have spent an extra couple of hours staring at the code, and the
whole looked fine, so applied. While on it, I have tested the new TAP
tests with all the possible combinations of --without-zlib and
--with-lz4.
--
Michael
