Re: Add LZ4 compression in pg_dump

From: Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>
To: gkokolatos(at)pm(dot)me, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Alexander Lakhin <exclusion(at)gmail(dot)com>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, shiy(dot)fnst(at)fujitsu(dot)com, pgsql-hackers(at)lists(dot)postgresql(dot)org, Rachel Heaton <rachelmheaton(at)gmail(dot)com>
Subject: Re: Add LZ4 compression in pg_dump
Date: 2023-05-08 18:20:42
Message-ID: 1963a93f-001d-aa60-dec8-a7e1c6e03203@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 5/8/23 18:19, gkokolatos(at)pm(dot)me wrote:
>
>
>
>
>
> ------- Original Message -------
> On Monday, May 8th, 2023 at 3:16 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
>
>>
>>
>> I wrote:
>>
>>> Michael Paquier michael(at)paquier(dot)xyz writes:
>>>
>>>> While testing this patch, I have triggered an error pointing out that
>>>> the decompression path of LZ4 is broken for table data. I can
>>>> reproduce that with a dump of the regression database, as of:
>>>> make installcheck
>>>> pg_dump --format=d --file=dump_lz4 --compress=lz4 regression
>>
>>> Ugh. Reproduced here ... so we need an open item for this.
>>
>>
>> BTW, it seems to work with --format=c.
>>
>
> Thank you for the extra tests. It seems that exists a gap in the test
> coverage. Please find a patch attached that is addressing the issue
> and attempt to provide tests for it.
>

Seems I'm getting messages with a delay - this is mostly the same fix I
ended up with, not realizing you already posted a fix.

I don't think we need the local "in" variable - the pointer parameter is
local in the function, so we can modify it directly (with a cast).
WriteDataToArchiveLZ4 does it that way too.

The tests are definitely a good idea. I wonder if we should add a
comment to DEFAULT_IO_BUFFER_SIZE mentioning that if we choose to
increase the value in the future, we needs to tweak the tests too to use
more data in order to exercise the buffering etc. Maybe it's obvious?

regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2023-05-08 19:11:17 Re: benchmark results comparing versions 15.2 and 16
Previous Message Tomas Vondra 2023-05-08 18:00:39 Re: Add LZ4 compression in pg_dump