Re: [REVIEW] Re: Compression of full-page-writes

Hi,

Attached patch adds new GUC parameter 'compress_backup_block'.
When this parameter is enabled, the server just compresses FPW
(full-page-writes) in WAL by using pglz_compress() before inserting it
to the WAL buffers. Then, the compressed FPW is decompressed
in recovery. This is very simple patch.

The purpose of this patch is the reduction of WAL size.
Under heavy write load, the server needs to write a large amount of
WAL and this is likely to be a bottleneck. What's the worse is,
in replication, a large amount of WAL would have harmful effect on
not only WAL writing in the master, but also WAL streaming and
WAL writing in the standby. Also we would need to spend more
money on the storage to store such a large data.
I'd like to alleviate such harmful situations by reducing WAL size.

My idea is very simple, just compress FPW because FPW is
a big part of WAL. I used pglz_compress() as a compression method,
but you might think that other method is better. We can add
something like FPW-compression-hook for that later. The patch
adds new GUC parameter, but I'm thinking to merge it to full_page_writes
parameter to avoid increasing the number of GUC. That is,
I'm thinking to change full_page_writes so that it can accept new value
'compress'.

I measured how much WAL this patch can reduce, by using pgbench.

* Server spec
CPU: 8core, Intel(R) Core(TM) i7-3630QM CPU @ 2.40GHz
Mem: 16GB
Disk: 500GB SSD Samsung 840

* Benchmark
pgbench -c 32 -j 4 -T 900 -M prepared
scaling factor: 100

checkpoint_segments = 1024
checkpoint_timeout = 5min
(every checkpoint during benchmark were triggered by checkpoint_timeout)

* Result
[tps]
1386.8 (compress_backup_block = off)
1627.7 (compress_backup_block = on)

[the amount of WAL generated during running pgbench]
4302 MB (compress_backup_block = off)
1521 MB (compress_backup_block = on)

At least in my test, the patch could reduce the WAL size to one-third!

The patch is WIP yet. But I'd like to hear the opinions about this idea
before completing it, and then add the patch to next CF if okay.

Regards,

--
Fujii Masao

(2013/08/30 11:55), Fujii Masao wrote:
> Hi,
>
> Attached patch adds new GUC parameter 'compress_backup_block'.
> When this parameter is enabled, the server just compresses FPW
> (full-page-writes) in WAL by using pglz_compress() before inserting it
> to the WAL buffers. Then, the compressed FPW is decompressed
> in recovery. This is very simple patch.
>
> The purpose of this patch is the reduction of WAL size.
> Under heavy write load, the server needs to write a large amount of
> WAL and this is likely to be a bottleneck. What's the worse is,
> in replication, a large amount of WAL would have harmful effect on
> not only WAL writing in the master, but also WAL streaming and
> WAL writing in the standby. Also we would need to spend more
> money on the storage to store such a large data.
> I'd like to alleviate such harmful situations by reducing WAL size.
>
> My idea is very simple, just compress FPW because FPW is
> a big part of WAL. I used pglz_compress() as a compression method,
> but you might think that other method is better. We can add
> something like FPW-compression-hook for that later. The patch
> adds new GUC parameter, but I'm thinking to merge it to full_page_writes
> parameter to avoid increasing the number of GUC. That is,
> I'm thinking to change full_page_writes so that it can accept new value
> 'compress'.
>
> I measured how much WAL this patch can reduce, by using pgbench.
>
> * Server spec
> CPU: 8core, Intel(R) Core(TM) i7-3630QM CPU @ 2.40GHz
> Mem: 16GB
> Disk: 500GB SSD Samsung 840
>
> * Benchmark
> pgbench -c 32 -j 4 -T 900 -M prepared
> scaling factor: 100
>
> checkpoint_segments = 1024
> checkpoint_timeout = 5min
> (every checkpoint during benchmark were triggered by checkpoint_timeout)

I believe that the amount of backup blocks in WAL files is affected
by how often the checkpoints are occurring, particularly under such
update-intensive workload.

Under your configuration, checkpoint should occur so often.
So, you need to change checkpoint_timeout larger in order to
determine whether the patch is realistic.

Regards,

>
> * Result
> [tps]
> 1386.8 (compress_backup_block = off)
> 1627.7 (compress_backup_block = on)
>
> [the amount of WAL generated during running pgbench]
> 4302 MB (compress_backup_block = off)
> 1521 MB (compress_backup_block = on)
>
> At least in my test, the patch could reduce the WAL size to one-third!
>
> The patch is WIP yet. But I'd like to hear the opinions about this idea
> before completing it, and then add the patch to next CF if okay.
>
> Regards,
>
>
>
>

--
Satoshi Nagayasu <snaga(at)uptime(dot)jp>
Uptime Technologies, LLC. http://www.uptime.jp

(2013/08/30 12:07), Satoshi Nagayasu wrote:
>
>
> (2013/08/30 11:55), Fujii Masao wrote:
>> Hi,
>>
>> Attached patch adds new GUC parameter 'compress_backup_block'.
>> When this parameter is enabled, the server just compresses FPW
>> (full-page-writes) in WAL by using pglz_compress() before inserting it
>> to the WAL buffers. Then, the compressed FPW is decompressed
>> in recovery. This is very simple patch.
>>
>> The purpose of this patch is the reduction of WAL size.
>> Under heavy write load, the server needs to write a large amount of
>> WAL and this is likely to be a bottleneck. What's the worse is,
>> in replication, a large amount of WAL would have harmful effect on
>> not only WAL writing in the master, but also WAL streaming and
>> WAL writing in the standby. Also we would need to spend more
>> money on the storage to store such a large data.
>> I'd like to alleviate such harmful situations by reducing WAL size.
>>
>> My idea is very simple, just compress FPW because FPW is
>> a big part of WAL. I used pglz_compress() as a compression method,
>> but you might think that other method is better. We can add
>> something like FPW-compression-hook for that later. The patch
>> adds new GUC parameter, but I'm thinking to merge it to full_page_writes
>> parameter to avoid increasing the number of GUC. That is,
>> I'm thinking to change full_page_writes so that it can accept new value
>> 'compress'.
>>
>> I measured how much WAL this patch can reduce, by using pgbench.
>>
>> * Server spec
>> CPU: 8core, Intel(R) Core(TM) i7-3630QM CPU @ 2.40GHz
>> Mem: 16GB
>> Disk: 500GB SSD Samsung 840
>>
>> * Benchmark
>> pgbench -c 32 -j 4 -T 900 -M prepared
>> scaling factor: 100
>>
>> checkpoint_segments = 1024
>> checkpoint_timeout = 5min
>> (every checkpoint during benchmark were triggered by
>> checkpoint_timeout)
>
> I believe that the amount of backup blocks in WAL files is affected
> by how often the checkpoints are occurring, particularly under such
> update-intensive workload.
>
> Under your configuration, checkpoint should occur so often.
> So, you need to change checkpoint_timeout larger in order to
> determine whether the patch is realistic.

In fact, the following chart shows that checkpoint_timeout=30min
also reduces WAL size to one-third, compared with 5min timeout,
in the pgbench experimentation.

https://www.oss.ecl.ntt.co.jp/ossc/oss/img/pglesslog_img02.jpg

Regards,

>
> Regards,
>
>>
>> * Result
>> [tps]
>> 1386.8 (compress_backup_block = off)
>> 1627.7 (compress_backup_block = on)
>>
>> [the amount of WAL generated during running pgbench]
>> 4302 MB (compress_backup_block = off)
>> 1521 MB (compress_backup_block = on)
>>
>> At least in my test, the patch could reduce the WAL size to one-third!
>>
>> The patch is WIP yet. But I'd like to hear the opinions about this idea
>> before completing it, and then add the patch to next CF if okay.
>>
>> Regards,
>>
>>
>>
>>
>

--
Satoshi Nagayasu <snaga(at)uptime(dot)jp>
Uptime Technologies, LLC. http://www.uptime.jp

On Thu, Aug 29, 2013 at 7:55 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> [the amount of WAL generated during running pgbench]
> 4302 MB (compress_backup_block = off)
> 1521 MB (compress_backup_block = on)

Interesting.

I wonder, what is the impact on recovery time under the same
conditions? I suppose that the cost of the random I/O involved would
probably dominate just as with compress_backup_block = off. That said,
you've used an SSD here, so perhaps not.

--
Peter Geoghegan

On Fri, Aug 30, 2013 at 8:25 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> Hi,
>
> Attached patch adds new GUC parameter 'compress_backup_block'.
> When this parameter is enabled, the server just compresses FPW
> (full-page-writes) in WAL by using pglz_compress() before inserting it
> to the WAL buffers. Then, the compressed FPW is decompressed
> in recovery. This is very simple patch.
>
> The purpose of this patch is the reduction of WAL size.
> Under heavy write load, the server needs to write a large amount of
> WAL and this is likely to be a bottleneck. What's the worse is,
> in replication, a large amount of WAL would have harmful effect on
> not only WAL writing in the master, but also WAL streaming and
> WAL writing in the standby. Also we would need to spend more
> money on the storage to store such a large data.
> I'd like to alleviate such harmful situations by reducing WAL size.
>
> My idea is very simple, just compress FPW because FPW is
> a big part of WAL. I used pglz_compress() as a compression method,
> but you might think that other method is better. We can add
> something like FPW-compression-hook for that later. The patch
> adds new GUC parameter, but I'm thinking to merge it to full_page_writes
> parameter to avoid increasing the number of GUC. That is,
> I'm thinking to change full_page_writes so that it can accept new value
> 'compress'.
>
> I measured how much WAL this patch can reduce, by using pgbench.
>
> * Server spec
> CPU: 8core, Intel(R) Core(TM) i7-3630QM CPU @ 2.40GHz
> Mem: 16GB
> Disk: 500GB SSD Samsung 840
>
> * Benchmark
> pgbench -c 32 -j 4 -T 900 -M prepared
> scaling factor: 100
>
> checkpoint_segments = 1024
> checkpoint_timeout = 5min
> (every checkpoint during benchmark were triggered by checkpoint_timeout)
>
> * Result
> [tps]
> 1386.8 (compress_backup_block = off)
> 1627.7 (compress_backup_block = on)
>
> [the amount of WAL generated during running pgbench]
> 4302 MB (compress_backup_block = off)
> 1521 MB (compress_backup_block = on)

This is really nice data.

I think if you want, you can once try with one of the tests Heikki has
posted for one of my other patch which is here:
http://www.postgresql.org/message-id/51366323.8070606@vmware.com

Also if possible, for with lesser clients (1,2,4) and may be with more
frequency of checkpoint.

This is just to show benefits of this idea with other kind of workload.

I think we can do these tests later as well, I had mentioned because
sometime back (probably 6 months), one of my colleagues have tried
exactly the same idea of using compression method (LZ and few others)
for FPW, but it turned out that even though the WAL size is reduced
but performance went down which is not the case in the data you have
shown even though you have used SSD, might be he has done some mistake
as he was not as experienced, but I think still it's good to check on
various workloads.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

(2013/08/30 11:55), Fujii Masao wrote:
> * Benchmark
> pgbench -c 32 -j 4 -T 900 -M prepared
> scaling factor: 100
>
> checkpoint_segments = 1024
> checkpoint_timeout = 5min
> (every checkpoint during benchmark were triggered by checkpoint_timeout)
Did you execute munual checkpoint before starting benchmark?
We read only your message, it occuered three times checkpoint during benchmark.
But if you did not executed manual checkpoint, it would be different.

You had better clear this point for more transparent evaluation.

Regards,
--
Mitsumasa KONDO
NTT Open Software Center

Hi Fujii-san,

I must be missing something really trivial, but why not try to compress all
types of WAL blocks and not just FPW?

Regards,
Nikhils

On Fri, Aug 30, 2013 at 8:25 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:

> Hi,
>
> Attached patch adds new GUC parameter 'compress_backup_block'.
> When this parameter is enabled, the server just compresses FPW
> (full-page-writes) in WAL by using pglz_compress() before inserting it
> to the WAL buffers. Then, the compressed FPW is decompressed
> in recovery. This is very simple patch.
>
> The purpose of this patch is the reduction of WAL size.
> Under heavy write load, the server needs to write a large amount of
> WAL and this is likely to be a bottleneck. What's the worse is,
> in replication, a large amount of WAL would have harmful effect on
> not only WAL writing in the master, but also WAL streaming and
> WAL writing in the standby. Also we would need to spend more
> money on the storage to store such a large data.
> I'd like to alleviate such harmful situations by reducing WAL size.
>
> My idea is very simple, just compress FPW because FPW is
> a big part of WAL. I used pglz_compress() as a compression method,
> but you might think that other method is better. We can add
> something like FPW-compression-hook for that later. The patch
> adds new GUC parameter, but I'm thinking to merge it to full_page_writes
> parameter to avoid increasing the number of GUC. That is,
> I'm thinking to change full_page_writes so that it can accept new value
> 'compress'.
>
> I measured how much WAL this patch can reduce, by using pgbench.
>
> * Server spec
> CPU: 8core, Intel(R) Core(TM) i7-3630QM CPU @ 2.40GHz
> Mem: 16GB
> Disk: 500GB SSD Samsung 840
>
> * Benchmark
> pgbench -c 32 -j 4 -T 900 -M prepared
> scaling factor: 100
>
> checkpoint_segments = 1024
> checkpoint_timeout = 5min
> (every checkpoint during benchmark were triggered by checkpoint_timeout)
>
> * Result
> [tps]
> 1386.8 (compress_backup_block = off)
> 1627.7 (compress_backup_block = on)
>
> [the amount of WAL generated during running pgbench]
> 4302 MB (compress_backup_block = off)
> 1521 MB (compress_backup_block = on)
>
> At least in my test, the patch could reduce the WAL size to one-third!
>
> The patch is WIP yet. But I'd like to hear the opinions about this idea
> before completing it, and then add the patch to next CF if okay.
>
> Regards,
>
> --
> Fujii Masao
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>

On Fri, Aug 30, 2013 at 11:55 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> My idea is very simple, just compress FPW because FPW is
> a big part of WAL. I used pglz_compress() as a compression method,
> but you might think that other method is better. We can add
> something like FPW-compression-hook for that later. The patch
> adds new GUC parameter, but I'm thinking to merge it to full_page_writes
> parameter to avoid increasing the number of GUC. That is,
> I'm thinking to change full_page_writes so that it can accept new value
> 'compress'.
Instead of a generic 'compress', what about using the name of the
compression method as parameter value? Just to keep the door open to
new types of compression methods.

> * Result
> [tps]
> 1386.8 (compress_backup_block = off)
> 1627.7 (compress_backup_block = on)
>
> [the amount of WAL generated during running pgbench]
> 4302 MB (compress_backup_block = off)
> 1521 MB (compress_backup_block = on)
>
> At least in my test, the patch could reduce the WAL size to one-third!
Nice numbers! Testing this patch with other benchmarks than pgbench
would be interesting as well.
--
Michael

On Fri, Aug 30, 2013 at 12:43 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> On Thu, Aug 29, 2013 at 7:55 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> [the amount of WAL generated during running pgbench]
>> 4302 MB (compress_backup_block = off)
>> 1521 MB (compress_backup_block = on)
>
> Interesting.
>
> I wonder, what is the impact on recovery time under the same
> conditions?

Will test! I can imagine that the recovery time would be a bit
longer with compress_backup_block=on because compressed
FPW needs to be decompressed.

> I suppose that the cost of the random I/O involved would
> probably dominate just as with compress_backup_block = off. That said,
> you've used an SSD here, so perhaps not.

Oh, maybe my description was confusing. full_page_writes was enabled
while running the benchmark even if compress_backup_block = off.
I've not merged those two parameters yet. So even in
compress_backup_block = off, random I/O would not be increased in recovery.

Regards,

--
Fujii Masao

On Fri, Aug 30, 2013 at 1:43 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Fri, Aug 30, 2013 at 8:25 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> Hi,
>>
>> Attached patch adds new GUC parameter 'compress_backup_block'.
>> When this parameter is enabled, the server just compresses FPW
>> (full-page-writes) in WAL by using pglz_compress() before inserting it
>> to the WAL buffers. Then, the compressed FPW is decompressed
>> in recovery. This is very simple patch.
>>
>> The purpose of this patch is the reduction of WAL size.
>> Under heavy write load, the server needs to write a large amount of
>> WAL and this is likely to be a bottleneck. What's the worse is,
>> in replication, a large amount of WAL would have harmful effect on
>> not only WAL writing in the master, but also WAL streaming and
>> WAL writing in the standby. Also we would need to spend more
>> money on the storage to store such a large data.
>> I'd like to alleviate such harmful situations by reducing WAL size.
>>
>> My idea is very simple, just compress FPW because FPW is
>> a big part of WAL. I used pglz_compress() as a compression method,
>> but you might think that other method is better. We can add
>> something like FPW-compression-hook for that later. The patch
>> adds new GUC parameter, but I'm thinking to merge it to full_page_writes
>> parameter to avoid increasing the number of GUC. That is,
>> I'm thinking to change full_page_writes so that it can accept new value
>> 'compress'.
>>
>> I measured how much WAL this patch can reduce, by using pgbench.
>>
>> * Server spec
>> CPU: 8core, Intel(R) Core(TM) i7-3630QM CPU @ 2.40GHz
>> Mem: 16GB
>> Disk: 500GB SSD Samsung 840
>>
>> * Benchmark
>> pgbench -c 32 -j 4 -T 900 -M prepared
>> scaling factor: 100
>>
>> checkpoint_segments = 1024
>> checkpoint_timeout = 5min
>> (every checkpoint during benchmark were triggered by checkpoint_timeout)
>>
>> * Result
>> [tps]
>> 1386.8 (compress_backup_block = off)
>> 1627.7 (compress_backup_block = on)
>>
>> [the amount of WAL generated during running pgbench]
>> 4302 MB (compress_backup_block = off)
>> 1521 MB (compress_backup_block = on)
>
> This is really nice data.
>
> I think if you want, you can once try with one of the tests Heikki has
> posted for one of my other patch which is here:
> http://www.postgresql.org/message-id/51366323.8070606@vmware.com
>
> Also if possible, for with lesser clients (1,2,4) and may be with more
> frequency of checkpoint.
>
> This is just to show benefits of this idea with other kind of workload.

Yep, I will do more tests.

> I think we can do these tests later as well, I had mentioned because
> sometime back (probably 6 months), one of my colleagues have tried
> exactly the same idea of using compression method (LZ and few others)
> for FPW, but it turned out that even though the WAL size is reduced
> but performance went down which is not the case in the data you have
> shown even though you have used SSD, might be he has done some mistake
> as he was not as experienced, but I think still it's good to check on
> various workloads.

I'd appreciate if you test the patch with HDD. Now I have no machine with HDD.

Regards,

--
Fujii Masao

On Fri, Aug 30, 2013 at 2:32 PM, KONDO Mitsumasa
<kondo(dot)mitsumasa(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> (2013/08/30 11:55), Fujii Masao wrote:
>>
>> * Benchmark
>> pgbench -c 32 -j 4 -T 900 -M prepared
>> scaling factor: 100
>>
>> checkpoint_segments = 1024
>> checkpoint_timeout = 5min
>> (every checkpoint during benchmark were triggered by
>> checkpoint_timeout)
>
> Did you execute munual checkpoint before starting benchmark?

Yes.

> We read only your message, it occuered three times checkpoint during
> benchmark.
> But if you did not executed manual checkpoint, it would be different.
>
> You had better clear this point for more transparent evaluation.

What I executed was:

-------------------------------------
CHECKPOINT
SELECT pg_current_xlog_location()
pgbench -c 32 -j 4 -T 900 -M prepared -r -P 10
SELECT pg_current_xlog_location()
SELECT pg_xlog_location_diff() -- calculate the diff of the above locations
-------------------------------------

I repeated this several times to eliminate the noise.

Regards,

--
Fujii Masao

On Thu, Aug 29, 2013 at 10:55 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> I suppose that the cost of the random I/O involved would
>> probably dominate just as with compress_backup_block = off. That said,
>> you've used an SSD here, so perhaps not.
>
> Oh, maybe my description was confusing. full_page_writes was enabled
> while running the benchmark even if compress_backup_block = off.
> I've not merged those two parameters yet. So even in
> compress_backup_block = off, random I/O would not be increased in recovery.

I understood it that way. I just meant that it could be that the
random I/O was so expensive that the additional cost of decompressing
the FPIs looked insignificant in comparison. If that was the case, the
increase in recovery time would be modest.

--
Peter Geoghegan

On Fri, Aug 30, 2013 at 2:37 PM, Nikhil Sontakke <nikkhils(at)gmail(dot)com> wrote:
> Hi Fujii-san,
>
> I must be missing something really trivial, but why not try to compress all
> types of WAL blocks and not just FPW?

The size of non-FPW WAL is small, compared to that of FPW.
I thought that compression of such a small WAL would not have
big effect on the reduction of WAL size. Rather, compression of
every WAL records might cause large performance overhead.

Also, focusing on FPW makes the patch very simple. We can
add the compression of other WAL later if we want.

Regards,

--
Fujii Masao

On 30.08.2013 05:55, Fujii Masao wrote:
> * Result
> [tps]
> 1386.8 (compress_backup_block = off)
> 1627.7 (compress_backup_block = on)

It would be good to check how much of this effect comes from reducing
the amount of data that needs to be CRC'd, because there has been some
talk of replacing the current CRC-32 algorithm with something faster.
See
http://www.postgresql.org/message-id/20130829223004.GD4283@awork2.anarazel.de.
It might even be beneficial to use one routine for full-page-writes,
which are generally much larger than other WAL records, and another
routine for smaller records. As long as they both produce the same CRC,
of course.

Speeding up the CRC calculation obviously won't help with the WAL volume
per se, ie. you still generate the same amount of WAL that needs to be
shipped in replication. But then again, if all you want to do is to
reduce the volume, you could just compress the whole WAL stream.

- Heikki

On Thu, Aug 29, 2013 at 10:55 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> Attached patch adds new GUC parameter 'compress_backup_block'.

I think this is a great idea.

(This is not to disagree with any of the suggestions made on this
thread for further investigation, all of which I think I basically
agree with. But I just wanted to voice general support for the
general idea, regardless of what specifically we end up with.)

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, Aug 30, 2013 at 11:55 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> Hi,
>
> Attached patch adds new GUC parameter 'compress_backup_block'.
> When this parameter is enabled, the server just compresses FPW
> (full-page-writes) in WAL by using pglz_compress() before inserting it
> to the WAL buffers. Then, the compressed FPW is decompressed
> in recovery. This is very simple patch.
>
> The purpose of this patch is the reduction of WAL size.
> Under heavy write load, the server needs to write a large amount of
> WAL and this is likely to be a bottleneck. What's the worse is,
> in replication, a large amount of WAL would have harmful effect on
> not only WAL writing in the master, but also WAL streaming and
> WAL writing in the standby. Also we would need to spend more
> money on the storage to store such a large data.
> I'd like to alleviate such harmful situations by reducing WAL size.
>
> My idea is very simple, just compress FPW because FPW is
> a big part of WAL. I used pglz_compress() as a compression method,
> but you might think that other method is better. We can add
> something like FPW-compression-hook for that later. The patch
> adds new GUC parameter, but I'm thinking to merge it to full_page_writes
> parameter to avoid increasing the number of GUC. That is,
> I'm thinking to change full_page_writes so that it can accept new value
> 'compress'.

Done. Attached is the updated version of the patch.

In this patch, full_page_writes accepts three values: on, compress, and off.
When it's set to compress, the full page image is compressed before it's
inserted into the WAL buffers.

I measured how much this patch affects the performance and the WAL
volume again, and I also measured how much this patch affects the
recovery time.

* Server spec
CPU: 8core, Intel(R) Core(TM) i7-3630QM CPU @ 2.40GHz
Mem: 16GB
Disk: 500GB SSD Samsung 840

* Benchmark
pgbench -c 32 -j 4 -T 900 -M prepared
scaling factor: 100

checkpoint_segments = 1024
checkpoint_timeout = 5min
(every checkpoint during benchmark were triggered by checkpoint_timeout)

* Result
[tps]
1344.2 (full_page_writes = on)
1605.9 (compress)
1810.1 (off)

[the amount of WAL generated during running pgbench]
4422 MB (on)
1517 MB (compress)
885 MB (off)

[time required to replay WAL generated during running pgbench]
61s (on) .... 1209911 transactions were replayed,
recovery speed: 19834.6 transactions/sec
39s (compress) .... 1445446 transactions were replayed,
recovery speed: 37062.7 transactions/sec
37s (off) .... 1629235 transactions were replayed,
recovery speed: 44033.3 transactions/sec

When full_page_writes is disabled, the recovery speed is basically very low
because of random I/O. But, ISTM that, since I was using SSD in my box,
the recovery with full_page_writse=off was fastest.

Regards,

--
Fujii Masao

On 2013-09-11 19:39:14 +0900, Fujii Masao wrote:
> * Benchmark
> pgbench -c 32 -j 4 -T 900 -M prepared
> scaling factor: 100
>
> checkpoint_segments = 1024
> checkpoint_timeout = 5min
> (every checkpoint during benchmark were triggered by checkpoint_timeout)
>
> * Result
> [tps]
> 1344.2 (full_page_writes = on)
> 1605.9 (compress)
> 1810.1 (off)
>
> [the amount of WAL generated during running pgbench]
> 4422 MB (on)
> 1517 MB (compress)
> 885 MB (off)
>
> [time required to replay WAL generated during running pgbench]
> 61s (on) .... 1209911 transactions were replayed,
> recovery speed: 19834.6 transactions/sec
> 39s (compress) .... 1445446 transactions were replayed,
> recovery speed: 37062.7 transactions/sec
> 37s (off) .... 1629235 transactions were replayed,
> recovery speed: 44033.3 transactions/sec

ISTM for those benchmarks you should use an absolute number of
transactions, not one based on elapsed time. Otherwise the comparison
isn't really meaningful.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Wed, Sep 11, 2013 at 7:39 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> On Fri, Aug 30, 2013 at 11:55 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> Hi,
>>
>> Attached patch adds new GUC parameter 'compress_backup_block'.
>> When this parameter is enabled, the server just compresses FPW
>> (full-page-writes) in WAL by using pglz_compress() before inserting it
>> to the WAL buffers. Then, the compressed FPW is decompressed
>> in recovery. This is very simple patch.
>>
>> The purpose of this patch is the reduction of WAL size.
>> Under heavy write load, the server needs to write a large amount of
>> WAL and this is likely to be a bottleneck. What's the worse is,
>> in replication, a large amount of WAL would have harmful effect on
>> not only WAL writing in the master, but also WAL streaming and
>> WAL writing in the standby. Also we would need to spend more
>> money on the storage to store such a large data.
>> I'd like to alleviate such harmful situations by reducing WAL size.
>>
>> My idea is very simple, just compress FPW because FPW is
>> a big part of WAL. I used pglz_compress() as a compression method,
>> but you might think that other method is better. We can add
>> something like FPW-compression-hook for that later. The patch
>> adds new GUC parameter, but I'm thinking to merge it to full_page_writes
>> parameter to avoid increasing the number of GUC. That is,
>> I'm thinking to change full_page_writes so that it can accept new value
>> 'compress'.
>
> Done. Attached is the updated version of the patch.
>
> In this patch, full_page_writes accepts three values: on, compress, and off.
> When it's set to compress, the full page image is compressed before it's
> inserted into the WAL buffers.
>
> I measured how much this patch affects the performance and the WAL
> volume again, and I also measured how much this patch affects the
> recovery time.
>
> * Server spec
> CPU: 8core, Intel(R) Core(TM) i7-3630QM CPU @ 2.40GHz
> Mem: 16GB
> Disk: 500GB SSD Samsung 840
>
> * Benchmark
> pgbench -c 32 -j 4 -T 900 -M prepared
> scaling factor: 100
>
> checkpoint_segments = 1024
> checkpoint_timeout = 5min
> (every checkpoint during benchmark were triggered by checkpoint_timeout)
>
> * Result
> [tps]
> 1344.2 (full_page_writes = on)
> 1605.9 (compress)
> 1810.1 (off)
>
> [the amount of WAL generated during running pgbench]
> 4422 MB (on)
> 1517 MB (compress)
> 885 MB (off)

On second thought, the patch could compress WAL very much because I
used pgbench.
Most of data in pgbench are pgbench_accounts table's "filler" columns, i.e.,
blank-padded empty strings. So, the compression ratio of WAL was very high.

I will do the same measurement by using another benchmark.

Regards,

--
Fujii Masao

Hi Fujii-san,

(2013/09/30 12:49), Fujii Masao wrote:
> On second thought, the patch could compress WAL very much because I used pgbench.
> I will do the same measurement by using another benchmark.
If you hope, I can test this patch in DBT-2 benchmark in end of this week.
I will use under following test server.

* Test server
Server: HP Proliant DL360 G7
CPU: Xeon E5640 2.66GHz (1P/4C)
Memory: 18GB(PC3-10600R-9)
Disk: 146GB(15k)*4 RAID1+0
RAID controller: P410i/256MB

This is PG-REX test server as you know.

Regards,
--
Mitsumasa KONDO
NTT Open Source Software Center

On Mon, Sep 30, 2013 at 1:27 PM, KONDO Mitsumasa
<kondo(dot)mitsumasa(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> Hi Fujii-san,
>
>
> (2013/09/30 12:49), Fujii Masao wrote:
>> On second thought, the patch could compress WAL very much because I used
>> pgbench.
>>
>> I will do the same measurement by using another benchmark.
>
> If you hope, I can test this patch in DBT-2 benchmark in end of this week.
> I will use under following test server.
>
> * Test server
> Server: HP Proliant DL360 G7
> CPU: Xeon E5640 2.66GHz (1P/4C)
> Memory: 18GB(PC3-10600R-9)
> Disk: 146GB(15k)*4 RAID1+0
> RAID controller: P410i/256MB

Yep, please! It's really helpful!

Regards,

--
Fujii Masao

On Mon, Sep 30, 2013 at 10:04 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> On Mon, Sep 30, 2013 at 1:27 PM, KONDO Mitsumasa
> <kondo(dot)mitsumasa(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>> Hi Fujii-san,
>>
>>
>> (2013/09/30 12:49), Fujii Masao wrote:
>>> On second thought, the patch could compress WAL very much because I used
>>> pgbench.
>>>
>>> I will do the same measurement by using another benchmark.
>>
>> If you hope, I can test this patch in DBT-2 benchmark in end of this week.
>> I will use under following test server.
>>
>> * Test server
>> Server: HP Proliant DL360 G7
>> CPU: Xeon E5640 2.66GHz (1P/4C)
>> Memory: 18GB(PC3-10600R-9)
>> Disk: 146GB(15k)*4 RAID1+0
>> RAID controller: P410i/256MB
>
> Yep, please! It's really helpful!

I think it will be useful if you can get the data for 1 and 2 threads
(may be with pgbench itself) as well, because the WAL reduction is
almost sure, but the only thing is that it should not dip tps in some
of the scenarios.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

(2013/09/30 13:55), Amit Kapila wrote:
> On Mon, Sep 30, 2013 at 10:04 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> Yep, please! It's really helpful!
OK! I test with single instance and synchronous replication constitution.

By the way, you posted patch which is sync_file_range() WAL writing method in 3
years ago. I think it is also good for performance. As the reason, I read
sync_file_range() and fdatasync() in latest linux kernel code(3.9.11),
fdatasync() writes in dirty buffers of the whole file, on the other hand,
sync_file_range() writes in partial dirty buffers. In more detail, these
functions use the same function in kernel source code, fdatasync() is
vfs_fsync_range(file, 0, LLONG_MAX, 1), and sync_file_range() is
vfs_fsync_range(file, offset, amount, 1).
It is obvious that which is more efficiently in WAL writing.

You had better confirm it in linux kernel's git. I think your conviction will be
more deeply.
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/fs/sync.c?id=refs/tags/v3.11.2

> I think it will be useful if you can get the data for 1 and 2 threads
> (may be with pgbench itself) as well, because the WAL reduction is
> almost sure, but the only thing is that it should not dip tps in some
> of the scenarios.
That's right. I also want to know about this patch in MD environment, because
MD has strong point in sequential write which like WAL writing.

Regards,
--
Mitsumasa KONDO
NTT Open Source Software Center

On Mon, Sep 30, 2013 at 1:55 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Mon, Sep 30, 2013 at 10:04 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> On Mon, Sep 30, 2013 at 1:27 PM, KONDO Mitsumasa
>> <kondo(dot)mitsumasa(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>>> Hi Fujii-san,
>>>
>>>
>>> (2013/09/30 12:49), Fujii Masao wrote:
>>>> On second thought, the patch could compress WAL very much because I used
>>>> pgbench.
>>>>
>>>> I will do the same measurement by using another benchmark.
>>>
>>> If you hope, I can test this patch in DBT-2 benchmark in end of this week.
>>> I will use under following test server.
>>>
>>> * Test server
>>> Server: HP Proliant DL360 G7
>>> CPU: Xeon E5640 2.66GHz (1P/4C)
>>> Memory: 18GB(PC3-10600R-9)
>>> Disk: 146GB(15k)*4 RAID1+0
>>> RAID controller: P410i/256MB
>>
>> Yep, please! It's really helpful!
>
> I think it will be useful if you can get the data for 1 and 2 threads
> (may be with pgbench itself) as well, because the WAL reduction is
> almost sure, but the only thing is that it should not dip tps in some
> of the scenarios.

Here is the measurement result of pgbench with 1 thread.

scaling factor: 100
query mode: prepared
number of clients: 1
number of threads: 1
duration: 900 s

WAL Volume
- 1344 MB (full_page_writes = on)
- 349 MB (compress)
- 78 MB (off)

TPS
117.369221 (on)
143.908024 (compress)
163.722063 (off)

Regards,

--
Fujii Masao

On Fri, Oct 4, 2013 at 10:49 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> On Mon, Sep 30, 2013 at 1:55 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> On Mon, Sep 30, 2013 at 10:04 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>> On Mon, Sep 30, 2013 at 1:27 PM, KONDO Mitsumasa
>>> <kondo(dot)mitsumasa(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>>>> Hi Fujii-san,
>>>>
>>>>
>>>> (2013/09/30 12:49), Fujii Masao wrote:
>>>>> On second thought, the patch could compress WAL very much because I used
>>>>> pgbench.
>>>>>
>>>>> I will do the same measurement by using another benchmark.
>>>>
>>>> If you hope, I can test this patch in DBT-2 benchmark in end of this week.
>>>> I will use under following test server.
>>>>
>>>> * Test server
>>>> Server: HP Proliant DL360 G7
>>>> CPU: Xeon E5640 2.66GHz (1P/4C)
>>>> Memory: 18GB(PC3-10600R-9)
>>>> Disk: 146GB(15k)*4 RAID1+0
>>>> RAID controller: P410i/256MB
>>>
>>> Yep, please! It's really helpful!
>>
>> I think it will be useful if you can get the data for 1 and 2 threads
>> (may be with pgbench itself) as well, because the WAL reduction is
>> almost sure, but the only thing is that it should not dip tps in some
>> of the scenarios.
>
> Here is the measurement result of pgbench with 1 thread.
>
> scaling factor: 100
> query mode: prepared
> number of clients: 1
> number of threads: 1
> duration: 900 s
>
> WAL Volume
> - 1344 MB (full_page_writes = on)
> - 349 MB (compress)
> - 78 MB (off)
>
> TPS
> 117.369221 (on)
> 143.908024 (compress)
> 163.722063 (off)

This data is good.
I will check if with the help of my old colleagues, I can get the
performance data on m/c where we have tried similar idea.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On 05 October 2013 17:12 Amit Kapila wrote:
>On Fri, Oct 4, 2013 at 10:49 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> On Mon, Sep 30, 2013 at 1:55 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>>> On Mon, Sep 30, 2013 at 10:04 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>>> On Mon, Sep 30, 2013 at 1:27 PM, KONDO Mitsumasa
>>>> <kondo(dot)mitsumasa(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>>>>> Hi Fujii-san,
>>>>>
>>>>>
>>>>> (2013/09/30 12:49), Fujii Masao wrote:
>>>>>> On second thought, the patch could compress WAL very much because
>>>>>> I used pgbench.
>>>>>>
>>>>>> I will do the same measurement by using another benchmark.
>>>>>
>>>>> If you hope, I can test this patch in DBT-2 benchmark in end of this week.
>>>>> I will use under following test server.
>>>>>
>>>>> * Test server
>>>>> Server: HP Proliant DL360 G7
>>>>> CPU: Xeon E5640 2.66GHz (1P/4C)
>>>>> Memory: 18GB(PC3-10600R-9)
>>>>> Disk: 146GB(15k)*4 RAID1+0
>>>>> RAID controller: P410i/256MB
>>>>
>>>> Yep, please! It's really helpful!
>>>
>>> I think it will be useful if you can get the data for 1 and 2 threads
>>> (may be with pgbench itself) as well, because the WAL reduction is
>>> almost sure, but the only thing is that it should not dip tps in some
>>> of the scenarios.
>>
>> Here is the measurement result of pgbench with 1 thread.
>>
>> scaling factor: 100
>> query mode: prepared
>> number of clients: 1
>> number of threads: 1
>> duration: 900 s
>>
>> WAL Volume
>> - 1344 MB (full_page_writes = on)
>> - 349 MB (compress)
>> - 78 MB (off)
>>
>> TPS
>> 117.369221 (on)
>> 143.908024 (compress)
>> 163.722063 (off)

>This data is good.
>I will check if with the help of my old colleagues, I can get the performance data on m/c where we have tried similar idea.

Thread-1 Threads-2
Head code FPW compress Head code FPW compress
Pgbench-org 5min 1011(0.96GB) 815(0.20GB) 2083(1.24GB) 1843(0.40GB)
Pgbench-1000 5min 958(1.16GB) 778(0.24GB) 1937(2.80GB) 1659(0.73GB)
Pgbench-org 15min 1065(1.43GB) 983(0.56GB) 2094(1.93GB) 2025(1.09GB)
Pgbench-1000 15min 1020(3.70GB) 898(1.05GB) 1383(5.31GB) 1908(2.49GB)

Pgbench-org - original pgbench
Pgbench-1000 - changed pgbench with a record size of 1000.
5 min - pgbench test carried out for 5 min.
15 min - pgbench test carried out for 15 min.

The checkpoint_timeout and checkpoint_segments are increased to make sure no checkpoint happens during the test run.

From the above readings it is observed that,
1. There a performance dip in one or two threads test, the amount of dip reduces with the test run time.
2. For two threads pgbench-1000 record size test, the fpw compress performance is good in 15min run.
3. More than 50% WAL reduction in all scenarios.

All these readings are measured with pgbench query mode as simple.
Please find the attached sheet for more details regarding machine and test configuration.

Regards,
Hari babu.

On 2013-09-11 12:43:21 +0200, Andres Freund wrote:
> On 2013-09-11 19:39:14 +0900, Fujii Masao wrote:
> > * Benchmark
> > pgbench -c 32 -j 4 -T 900 -M prepared
> > scaling factor: 100
> >
> > checkpoint_segments = 1024
> > checkpoint_timeout = 5min
> > (every checkpoint during benchmark were triggered by checkpoint_timeout)
> >
> > * Result
> > [tps]
> > 1344.2 (full_page_writes = on)
> > 1605.9 (compress)
> > 1810.1 (off)
> >
> > [the amount of WAL generated during running pgbench]
> > 4422 MB (on)
> > 1517 MB (compress)
> > 885 MB (off)
> >
> > [time required to replay WAL generated during running pgbench]
> > 61s (on) .... 1209911 transactions were replayed,
> > recovery speed: 19834.6 transactions/sec
> > 39s (compress) .... 1445446 transactions were replayed,
> > recovery speed: 37062.7 transactions/sec
> > 37s (off) .... 1629235 transactions were replayed,
> > recovery speed: 44033.3 transactions/sec
>
> ISTM for those benchmarks you should use an absolute number of
> transactions, not one based on elapsed time. Otherwise the comparison
> isn't really meaningful.

I really think we need to see recovery time benchmarks with a constant
amount of transactions to judge this properly.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

(2013/10/08 17:33), Haribabu kommi wrote:
> The checkpoint_timeout and checkpoint_segments are increased to make sure no checkpoint happens during the test run.
Your setting is easy occurred checkpoint in checkpoint_segments = 256. I don't
know number of disks in your test server, in my test server which has 4 magnetic
disk(1.5k rpm), postgres generates 50 - 100 WALs per minutes.

And I cannot understand your setting which is sync_commit = off. This setting
tend to cause cpu bottle-neck and data-loss. It is not general in database usage.
Therefore, your test is not fair comparison for Fujii's patch.

Going back to my DBT-2 benchmark, I have not got good performance (almost same
performance). So I am checking hunk, my setting, or something wrong in Fujii's
patch now. I am going to try to send test result tonight.

Regards,
--
Mitsumasa KONDO
NTT Open Source Software Center

On 08 October 2013 15:22 KONDO Mitsumasa wrote:
> (2013/10/08 17:33), Haribabu kommi wrote:
>> The checkpoint_timeout and checkpoint_segments are increased to make sure no checkpoint happens during the test run.

>Your setting is easy occurred checkpoint in checkpoint_segments = 256. I don't know number of disks in your test server, in my test server which has 4 magnetic disk(1.5k rpm), postgres generates 50 - 100 WALs per minutes.

A manual checkpoint is executed before starting of the test and verified as no checkpoint happened during the run by increasing the "checkpoint_warning".

>And I cannot understand your setting which is sync_commit = off. This setting tend to cause cpu bottle-neck and data-loss. It is not general in database usage.
Therefore, your test is not fair comparison for Fujii's patch.

I chosen the sync_commit=off mode because it generates more tps, thus it increases the volume of WAL.
I will test with sync_commit=on mode and provide the test results.

Regards,
Hari babu.

Hi,

I tested dbt-2 benchmark in single instance and synchronous replication.
Unfortunately, my benchmark results were not seen many differences...

* Test server
Server: HP Proliant DL360 G7
CPU: Xeon E5640 2.66GHz (1P/4C)
Memory: 18GB(PC3-10600R-9)
Disk: 146GB(15k)*4 RAID1+0
RAID controller: P410i/256MB

* Result
** Single instance**
| NOTPM | 90%tile | Average | S.Deviation
------------+-----------+-------------+---------+-------------
no-patched | 3322.93 | 20.469071 | 5.882 | 10.478
patched | 3315.42 | 19.086105 | 5.669 | 9.108

** Synchronous Replication **
| NOTPM | 90%tile | Average | S.Deviation
------------+-----------+-------------+---------+-------------
no-patched | 3275.55 | 21.332866 | 6.072 | 9.882
patched | 3318.82 | 18.141807 | 5.757 | 9.829

** Detail of result
http://pgstatsinfo.projects.pgfoundry.org/DBT-2_Fujii_patch/

I set full_page_write = compress with Fujii's patch in DBT-2. But it does not
seems to effect for eleminating WAL files. I will try to DBT-2 benchmark more
once, and try to normal pgbench in my test server.

Regards,
--
Mitsumasa KONDO
NTT Open Source Software Center

(2013/10/08 20:13), Haribabu kommi wrote:
> I chosen the sync_commit=off mode because it generates more tps, thus it increases the volume of WAL.
I did not think to there. Sorry...

> I will test with sync_commit=on mode and provide the test results.
OK. Thanks!

--
Mitsumasa KONDO
NTT Open Source Software Center

On 08 October 2013 18:42 KONDO Mitsumasa wrote:
>(2013/10/08 20:13), Haribabu kommi wrote:
>> I will test with sync_commit=on mode and provide the test results.
>OK. Thanks!

Pgbench test results with synchronous_commit mode as on.

Thread-1 Threads-2
Head code FPW compress Head code FPW compress
Pgbench-org 5min 138(0.24GB) 131(0.04GB) 160(0.28GB) 163(0.05GB)
Pgbench-1000 5min 140(0.29GB) 128(0.03GB) 160(0.33GB) 162(0.02GB)
Pgbench-org 15min 141(0.59GB) 136(0.12GB) 160(0.65GB) 162(0.14GB)
Pgbench-1000 15min 138(0.81GB) 134(0.11GB) 159(0.92GB) 162(0.18GB)

Pgbench-org - original pgbench
Pgbench-1000 - changed pgbench with a record size of 1000.
5 min - pgbench test carried out for 5 min.
15 min - pgbench test carried out for 15 min.

From the above readings it is observed that,
1. There a performance dip in one thread test, the amount of dip reduces with the test run time.
2. More than 75% WAL reduction in all scenarios.

Please find the attached sheet for more details regarding machine and test configuration

Regards,
Hari babu.

Hi,

I did a partial review of this patch, wherein I focused on the patch and
the code itself, as I saw other contributors already did some testing on
it, so that we know it applies cleanly and work to some good extend.

Fujii Masao <masao(dot)fujii(at)gmail(dot)com> writes:
> In this patch, full_page_writes accepts three values: on, compress, and off.
> When it's set to compress, the full page image is compressed before it's
> inserted into the WAL buffers.

Code review :

In full_page_writes_str() why are you returning "unrecognized" rather
than doing an ELOG(ERROR, …) for this unexpected situation?

The code switches to compression (or trying to) when the following
condition is met:

+ if (fpw <= FULL_PAGE_WRITES_COMPRESS)
+ {
+ rdt->data = CompressBackupBlock(page, BLCKSZ - bkpb->hole_length, &(rdt->len));

We have

+ typedef enum FullPageWritesLevel
+ {
+ FULL_PAGE_WRITES_OFF = 0,
+ FULL_PAGE_WRITES_COMPRESS,
+ FULL_PAGE_WRITES_ON
+ } FullPageWritesLevel;

+ #define FullPageWritesIsNeeded(fpw) (fpw >= FULL_PAGE_WRITES_COMPRESS)

I don't much like using the <= test against and ENUM and I'm not sure I
understand the intention you have here. It somehow looks like a typo and
disagrees with the macro. What about using the FullPageWritesIsNeeded
macro, and maybe rewriting the macro as

#define FullPageWritesIsNeeded(fpw) \
(fpw == FULL_PAGE_WRITES_COMPRESS || fpw == FULL_PAGE_WRITES_ON)

Also, having "on" imply "compress" is a little funny to me. Maybe we
should just finish our testing and be happy to always compress the full
page writes. What would the downside be exactly (on buzy IO system
writing less data even if needing more CPU will be the right trade-off).

I like that you're checking the savings of the compressed data with
respect to the uncompressed data and cancel the compression if there's
no gain. I wonder if your test accounts for enough padding and headers
though given the results we saw in other tests made in this thread.

Why do we have both the static function full_page_writes_str() and the
macro FullPageWritesStr, with two different implementations issuing
either "true" and "false" or "on" and "off"?

! unsigned hole_offset:15, /* number of bytes before "hole" */
! flags:2, /* state of a backup block, see below */
! hole_length:15; /* number of bytes in "hole" */

I don't understand that. I wanted to use that patch as a leverage to
smoothly discover the internals of our WAL system but won't have the
time to do that here. That said, I don't even know that C syntax.

+ #define BKPBLOCK_UNCOMPRESSED 0 /* uncompressed */
+ #define BKPBLOCK_COMPRESSED 1 /* comperssed */

There's a typo in the comment above.

> [time required to replay WAL generated during running pgbench]
> 61s (on) .... 1209911 transactions were replayed,
> recovery speed: 19834.6 transactions/sec
> 39s (compress) .... 1445446 transactions were replayed,
> recovery speed: 37062.7 transactions/sec
> 37s (off) .... 1629235 transactions were replayed,
> recovery speed: 44033.3 transactions/sec

How did you get those numbers ? pg_basebackup before the test and
archiving, then a PITR maybe? Is it possible to do the same test with
the same number of transactions to replay, I guess using the -t
parameter rather than the -T one for this testing.

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

On Tue, Oct 8, 2013 at 10:07 PM, KONDO Mitsumasa
<kondo(dot)mitsumasa(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> Hi,
>
> I tested dbt-2 benchmark in single instance and synchronous replication.

Thanks!

> Unfortunately, my benchmark results were not seen many differences...
>
>
> * Test server
> Server: HP Proliant DL360 G7
> CPU: Xeon E5640 2.66GHz (1P/4C)
> Memory: 18GB(PC3-10600R-9)
> Disk: 146GB(15k)*4 RAID1+0
> RAID controller: P410i/256MB
>
> * Result
> ** Single instance**
> | NOTPM | 90%tile | Average | S.Deviation
> ------------+-----------+-------------+---------+-------------
> no-patched | 3322.93 | 20.469071 | 5.882 | 10.478
> patched | 3315.42 | 19.086105 | 5.669 | 9.108
>
>
> ** Synchronous Replication **
> | NOTPM | 90%tile | Average | S.Deviation
> ------------+-----------+-------------+---------+-------------
> no-patched | 3275.55 | 21.332866 | 6.072 | 9.882
> patched | 3318.82 | 18.141807 | 5.757 | 9.829
>
> ** Detail of result
> http://pgstatsinfo.projects.pgfoundry.org/DBT-2_Fujii_patch/
>
>
> I set full_page_write = compress with Fujii's patch in DBT-2. But it does
> not
> seems to effect for eleminating WAL files.

Could you let me know how much WAL records were generated
during each benchmark?

I think that this benchmark result clearly means that the patch
has only limited effects in the reduction of WAL volume and
the performance improvement unless the database contains
highly-compressible data like pgbench_accounts.filler. But if
we can use other compression algorithm, maybe we can reduce
WAL volume very much. I'm not sure what algorithm is good
for WAL compression, though.

It might be better to introduce the hook for compression of FPW
so that users can freely use their compression module, rather
than just using pglz_compress(). Thought?

Regards,

--
Fujii Masao

On Wed, Oct 9, 2013 at 1:35 PM, Haribabu kommi
<haribabu(dot)kommi(at)huawei(dot)com> wrote:
> On 08 October 2013 18:42 KONDO Mitsumasa wrote:
>>(2013/10/08 20:13), Haribabu kommi wrote:
>>> I will test with sync_commit=on mode and provide the test results.
>>OK. Thanks!
>
> Pgbench test results with synchronous_commit mode as on.

Thanks!

> Thread-1 Threads-2
> Head code FPW compress Head code FPW compress
> Pgbench-org 5min 138(0.24GB) 131(0.04GB) 160(0.28GB) 163(0.05GB)
> Pgbench-1000 5min 140(0.29GB) 128(0.03GB) 160(0.33GB) 162(0.02GB)
> Pgbench-org 15min 141(0.59GB) 136(0.12GB) 160(0.65GB) 162(0.14GB)
> Pgbench-1000 15min 138(0.81GB) 134(0.11GB) 159(0.92GB) 162(0.18GB)
>
> Pgbench-org - original pgbench
> Pgbench-1000 - changed pgbench with a record size of 1000.

This means that you changed the data type of pgbench_accounts.filler
to char(1000)?

Regards,

--
Fujii Masao

On Fri, Oct 11, 2013 at 1:20 AM, Dimitri Fontaine
<dimitri(at)2ndquadrant(dot)fr> wrote:
> Hi,
>
> I did a partial review of this patch, wherein I focused on the patch and
> the code itself, as I saw other contributors already did some testing on
> it, so that we know it applies cleanly and work to some good extend.

Thanks a lot!

> In full_page_writes_str() why are you returning "unrecognized" rather
> than doing an ELOG(ERROR, …) for this unexpected situation?

It's because the similar functions 'wal_level_str' and 'dbState' also return
'unrecognized' in the unexpected situation. I just implemented
full_page_writes_str()
in the same manner.

If we do an elog(ERROR) in that case, pg_xlogdump would fail to dump
the 'broken' (i.e., unrecognized fpw is set) WAL file. I think that some
users want to use pg_xlogdump to investigate the broken WAL file, so
doing an elog(ERROR) seems not good to me.

> The code switches to compression (or trying to) when the following
> condition is met:
>
> + if (fpw <= FULL_PAGE_WRITES_COMPRESS)
> + {
> + rdt->data = CompressBackupBlock(page, BLCKSZ - bkpb->hole_length, &(rdt->len));
>
> We have
>
> + typedef enum FullPageWritesLevel
> + {
> + FULL_PAGE_WRITES_OFF = 0,
> + FULL_PAGE_WRITES_COMPRESS,
> + FULL_PAGE_WRITES_ON
> + } FullPageWritesLevel;
>
> + #define FullPageWritesIsNeeded(fpw) (fpw >= FULL_PAGE_WRITES_COMPRESS)
>
> I don't much like using the <= test against and ENUM and I'm not sure I
> understand the intention you have here. It somehow looks like a typo and
> disagrees with the macro.

I thought that FPW should be compressed only when full_page_writes is
set to 'compress' or 'off'. That is, 'off' implies a compression. When it's set
to 'off', FPW is basically not generated, so there is no need to call
CompressBackupBlock() in that case. But only during online base backup,
FPW is forcibly generated even when it's set to 'off'. So I used the check
"fpw <= FULL_PAGE_WRITES_COMPRESS" there.

> What about using the FullPageWritesIsNeeded
> macro, and maybe rewriting the macro as
>
> #define FullPageWritesIsNeeded(fpw) \
> (fpw == FULL_PAGE_WRITES_COMPRESS || fpw == FULL_PAGE_WRITES_ON)

I'm OK to change the macro so that the <= test is not used.

> Also, having "on" imply "compress" is a little funny to me. Maybe we
> should just finish our testing and be happy to always compress the full
> page writes. What would the downside be exactly (on buzy IO system
> writing less data even if needing more CPU will be the right trade-off).

"on" doesn't imply "compress". When full_page_writes is set to "on",
FPW is not compressed at all.

> I like that you're checking the savings of the compressed data with
> respect to the uncompressed data and cancel the compression if there's
> no gain. I wonder if your test accounts for enough padding and headers
> though given the results we saw in other tests made in this thread.

I'm afraid that the patch has only limited effects in WAL reduction and
performance improvement unless the database contains highly-compressible
data like large blank characters column. It really depends on the contents
of the database. So, obviously FPW compression should not be the default.
Maybe we can treat it as just tuning knob.

> Why do we have both the static function full_page_writes_str() and the
> macro FullPageWritesStr, with two different implementations issuing
> either "true" and "false" or "on" and "off"?

First I was thinking to use "on" and "off" because they are often used
as the setting value of boolean GUC. But unfortunately the existing
pg_xlogdump uses "true" and "false" to show the value of full_page_writes
in WAL. To avoid breaking the backward compatibility, I implmented
the "true/false" version of function. I'm really not sure how many people
want such a compatibility of pg_xlogdump, though.

> ! unsigned hole_offset:15, /* number of bytes before "hole" */
> ! flags:2, /* state of a backup block, see below */
> ! hole_length:15; /* number of bytes in "hole" */
>
> I don't understand that. I wanted to use that patch as a leverage to
> smoothly discover the internals of our WAL system but won't have the
> time to do that here.

We need the flag indicating whether each FPW is compressed or not.
If no such a flag exists in WAL, the standby cannot determine whether
it should decompress each FPW or not, and then cannot replay
the WAL containing FPW properly. That is, I just used a 'space' in
the header of FPW to have such a flag.

> That said, I don't even know that C syntax.

The struct 'ItemIdData' uses the same C syntax.

> + #define BKPBLOCK_UNCOMPRESSED 0 /* uncompressed */
> + #define BKPBLOCK_COMPRESSED 1 /* comperssed */
>
> There's a typo in the comment above.

Yep.

>> [time required to replay WAL generated during running pgbench]
>> 61s (on) .... 1209911 transactions were replayed,
>> recovery speed: 19834.6 transactions/sec
>> 39s (compress) .... 1445446 transactions were replayed,
>> recovery speed: 37062.7 transactions/sec
>> 37s (off) .... 1629235 transactions were replayed,
>> recovery speed: 44033.3 transactions/sec
>
> How did you get those numbers ? pg_basebackup before the test and
> archiving, then a PITR maybe? Is it possible to do the same test with
> the same number of transactions to replay, I guess using the -t
> parameter rather than the -T one for this testing.

Sure. To be honest, when I received the same request from Andres,
I did that benchmark. But unfortunately because of machine trouble,
I could not report it, yet. Will do that again.

Regards,

--
Fujii Masao

Hi,
On 2013-10-11 03:44:01 +0900, Fujii Masao wrote:
> I'm afraid that the patch has only limited effects in WAL reduction and
> performance improvement unless the database contains highly-compressible
> data like large blank characters column. It really depends on the contents
> of the database. So, obviously FPW compression should not be the default.
> Maybe we can treat it as just tuning knob.

Have you tried using lz4 (or snappy) instead of pglz? There's a patch
adding it to pg in
http://archives.postgresql.org/message-id/20130621000900.GA12425%40alap2.anarazel.de

If this really is only a benefit in scenarios with lots of such data, I
have to say I have my doubts about the benefits of the patch.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Fri, Oct 11, 2013 at 3:44 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> On Fri, Oct 11, 2013 at 1:20 AM, Dimitri Fontaine
> <dimitri(at)2ndquadrant(dot)fr> wrote:
>> Hi,
>>
>> I did a partial review of this patch, wherein I focused on the patch and
>> the code itself, as I saw other contributors already did some testing on
>> it, so that we know it applies cleanly and work to some good extend.
>
> Thanks a lot!
>
>> In full_page_writes_str() why are you returning "unrecognized" rather
>> than doing an ELOG(ERROR, …) for this unexpected situation?
>
> It's because the similar functions 'wal_level_str' and 'dbState' also return
> 'unrecognized' in the unexpected situation. I just implemented
> full_page_writes_str()
> in the same manner.
>
> If we do an elog(ERROR) in that case, pg_xlogdump would fail to dump
> the 'broken' (i.e., unrecognized fpw is set) WAL file. I think that some
> users want to use pg_xlogdump to investigate the broken WAL file, so
> doing an elog(ERROR) seems not good to me.
>
>> The code switches to compression (or trying to) when the following
>> condition is met:
>>
>> + if (fpw <= FULL_PAGE_WRITES_COMPRESS)
>> + {
>> + rdt->data = CompressBackupBlock(page, BLCKSZ - bkpb->hole_length, &(rdt->len));
>>
>> We have
>>
>> + typedef enum FullPageWritesLevel
>> + {
>> + FULL_PAGE_WRITES_OFF = 0,
>> + FULL_PAGE_WRITES_COMPRESS,
>> + FULL_PAGE_WRITES_ON
>> + } FullPageWritesLevel;
>>
>> + #define FullPageWritesIsNeeded(fpw) (fpw >= FULL_PAGE_WRITES_COMPRESS)
>>
>> I don't much like using the <= test against and ENUM and I'm not sure I
>> understand the intention you have here. It somehow looks like a typo and
>> disagrees with the macro.
>
> I thought that FPW should be compressed only when full_page_writes is
> set to 'compress' or 'off'. That is, 'off' implies a compression. When it's set
> to 'off', FPW is basically not generated, so there is no need to call
> CompressBackupBlock() in that case. But only during online base backup,
> FPW is forcibly generated even when it's set to 'off'. So I used the check
> "fpw <= FULL_PAGE_WRITES_COMPRESS" there.
>
>> What about using the FullPageWritesIsNeeded
>> macro, and maybe rewriting the macro as
>>
>> #define FullPageWritesIsNeeded(fpw) \
>> (fpw == FULL_PAGE_WRITES_COMPRESS || fpw == FULL_PAGE_WRITES_ON)
>
> I'm OK to change the macro so that the <= test is not used.
>
>> Also, having "on" imply "compress" is a little funny to me. Maybe we
>> should just finish our testing and be happy to always compress the full
>> page writes. What would the downside be exactly (on buzy IO system
>> writing less data even if needing more CPU will be the right trade-off).
>
> "on" doesn't imply "compress". When full_page_writes is set to "on",
> FPW is not compressed at all.
>
>> I like that you're checking the savings of the compressed data with
>> respect to the uncompressed data and cancel the compression if there's
>> no gain. I wonder if your test accounts for enough padding and headers
>> though given the results we saw in other tests made in this thread.
>
> I'm afraid that the patch has only limited effects in WAL reduction and
> performance improvement unless the database contains highly-compressible
> data like large blank characters column. It really depends on the contents
> of the database. So, obviously FPW compression should not be the default.
> Maybe we can treat it as just tuning knob.
>
>> Why do we have both the static function full_page_writes_str() and the
>> macro FullPageWritesStr, with two different implementations issuing
>> either "true" and "false" or "on" and "off"?
>
> First I was thinking to use "on" and "off" because they are often used
> as the setting value of boolean GUC. But unfortunately the existing
> pg_xlogdump uses "true" and "false" to show the value of full_page_writes
> in WAL. To avoid breaking the backward compatibility, I implmented
> the "true/false" version of function. I'm really not sure how many people
> want such a compatibility of pg_xlogdump, though.
>
>> ! unsigned hole_offset:15, /* number of bytes before "hole" */
>> ! flags:2, /* state of a backup block, see below */
>> ! hole_length:15; /* number of bytes in "hole" */
>>
>> I don't understand that. I wanted to use that patch as a leverage to
>> smoothly discover the internals of our WAL system but won't have the
>> time to do that here.
>
> We need the flag indicating whether each FPW is compressed or not.
> If no such a flag exists in WAL, the standby cannot determine whether
> it should decompress each FPW or not, and then cannot replay
> the WAL containing FPW properly. That is, I just used a 'space' in
> the header of FPW to have such a flag.
>
>> That said, I don't even know that C syntax.
>
> The struct 'ItemIdData' uses the same C syntax.
>
>> + #define BKPBLOCK_UNCOMPRESSED 0 /* uncompressed */
>> + #define BKPBLOCK_COMPRESSED 1 /* comperssed */
>>
>> There's a typo in the comment above.
>
> Yep.
>
>>> [time required to replay WAL generated during running pgbench]
>>> 61s (on) .... 1209911 transactions were replayed,
>>> recovery speed: 19834.6 transactions/sec
>>> 39s (compress) .... 1445446 transactions were replayed,
>>> recovery speed: 37062.7 transactions/sec
>>> 37s (off) .... 1629235 transactions were replayed,
>>> recovery speed: 44033.3 transactions/sec
>>
>> How did you get those numbers ? pg_basebackup before the test and
>> archiving, then a PITR maybe? Is it possible to do the same test with
>> the same number of transactions to replay, I guess using the -t
>> parameter rather than the -T one for this testing.
>
> Sure. To be honest, when I received the same request from Andres,
> I did that benchmark. But unfortunately because of machine trouble,
> I could not report it, yet. Will do that again.

Here is the benchmark result:

* Result
[tps]
1317.306391 (full_page_writes = on)
1628.407752 (compress)

[the amount of WAL generated during running pgbench]
1319 MB (on)
326 MB (compress)

[time required to replay WAL generated during running pgbench]
19s (on)
2013-10-11 12:05:09 JST LOG: redo starts at F/F1000028
2013-10-11 12:05:28 JST LOG: redo done at 10/446B7BF0

12s (on)
2013-10-11 12:06:22 JST LOG: redo starts at F/F1000028
2013-10-11 12:06:34 JST LOG: redo done at 10/446B7BF0

12s (on)
2013-10-11 12:07:19 JST LOG: redo starts at F/F1000028
2013-10-11 12:07:31 JST LOG: redo done at 10/446B7BF0

8s (compress)
2013-10-11 12:17:36 JST LOG: redo starts at 10/50000028
2013-10-11 12:17:44 JST LOG: redo done at 10/655AE478

8s (compress)
2013-10-11 12:18:26 JST LOG: redo starts at 10/50000028
2013-10-11 12:18:34 JST LOG: redo done at 10/655AE478

8s (compress)
2013-10-11 12:19:07 JST LOG: redo starts at 10/50000028
2013-10-11 12:19:15 JST LOG: redo done at 10/655AE478

[benchmark]
transaction type: TPC-B (sort of)
scaling factor: 100
query mode: prepared
number of clients: 32
number of threads: 4
number of transactions per client: 10000
number of transactions actually processed: 320000/320000

Regards,

--
Fujii Masao

On Fri, Oct 11, 2013 at 8:35 AM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> Hi,
> On 2013-10-11 03:44:01 +0900, Fujii Masao wrote:
>> I'm afraid that the patch has only limited effects in WAL reduction and
>> performance improvement unless the database contains highly-compressible
>> data like large blank characters column. It really depends on the contents
>> of the database. So, obviously FPW compression should not be the default.
>> Maybe we can treat it as just tuning knob.
>
>
> Have you tried using lz4 (or snappy) instead of pglz? There's a patch
> adding it to pg in
> http://archives.postgresql.org/message-id/20130621000900.GA12425%40alap2.anarazel.de

Yeah, it's worth checking them! Will do that.

> If this really is only a benefit in scenarios with lots of such data, I
> have to say I have my doubts about the benefits of the patch.

Yep, maybe the patch needs to be redesigned. Currently in the patch
compression is performed per FPW, i.e., the size of data to compress
is just 8KB. If we can increase the size of data to compress, we might
be able to improve the compression ratio. For example, by storing
all outstanding WAL data temporarily in local buffer, compressing them,
and then storing the compressed WAL data to WAL buffers.

Regards,

--
Fujii Masao

On Fri, Oct 11, 2013 at 5:05 AM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> Hi,
> On 2013-10-11 03:44:01 +0900, Fujii Masao wrote:
>> I'm afraid that the patch has only limited effects in WAL reduction and
>> performance improvement unless the database contains highly-compressible
>> data like large blank characters column. It really depends on the contents
>> of the database. So, obviously FPW compression should not be the default.
>> Maybe we can treat it as just tuning knob.
>
>
> Have you tried using lz4 (or snappy) instead of pglz? There's a patch
> adding it to pg in
> http://archives.postgresql.org/message-id/20130621000900.GA12425%40alap2.anarazel.de
>
> If this really is only a benefit in scenarios with lots of such data, I
> have to say I have my doubts about the benefits of the patch.

I think it will be difficult to prove by using any compression
algorithm, that it compresses in most of the scenario's.
In many cases it can so happen that the WAL will also not be reduced
and tps can also come down if the data is non-compressible, because
any compression algorithm will have to try to compress the data and it
will burn some cpu for that, which inturn will reduce tps.

As this patch is giving a knob to user to turn compression on/off, so
users can decide if they want such benefit.
Now some users can say that they have no idea, how or what kind of
data will be there in their databases, so such kind of users should
not use this option, but on the other side some users know that they
have similar pattern of data, so they can get benefit out of such
optimisations. For example in telecom industry, i have seen that they
have lot of data as CDR's (call data records) in their HLR databases
for which the data records will be different but of same pattern.

Being said above, I think both this patch and my patch "WAL reduction
for Update" (https://commitfest.postgresql.org/action/patch_view?id=1209)
are using same technique for WAL compression and can lead to similar
consequences in different ways.
So I suggest to have unified method to enable WAL Compression for both
the patches.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On 10 October 2013 23:06 Fujii Masao wrote:
>On Wed, Oct 9, 2013 at 1:35 PM, Haribabu kommi <haribabu(dot)kommi(at)huawei(dot)com> wrote:
>> Thread-1 Threads-2
>> Head code FPW compress Head code FPW compress
>> Pgbench-org 5min 138(0.24GB) 131(0.04GB) 160(0.28GB) 163(0.05GB)
>> Pgbench-1000 5min 140(0.29GB) 128(0.03GB) 160(0.33GB) 162(0.02GB)
>> Pgbench-org 15min 141(0.59GB) 136(0.12GB) 160(0.65GB) 162(0.14GB)
>> Pgbench-1000 15min 138(0.81GB) 134(0.11GB) 159(0.92GB) 162(0.18GB)
>>
>> Pgbench-org - original pgbench
>> Pgbench-1000 - changed pgbench with a record size of 1000.

>This means that you changed the data type of pgbench_accounts.filler to char(1000)?

Yes, I changed the filler column as char(1000).

Regards,
Hari babu.

On 2013-10-11 09:22:50 +0530, Amit Kapila wrote:
> I think it will be difficult to prove by using any compression
> algorithm, that it compresses in most of the scenario's.
> In many cases it can so happen that the WAL will also not be reduced
> and tps can also come down if the data is non-compressible, because
> any compression algorithm will have to try to compress the data and it
> will burn some cpu for that, which inturn will reduce tps.

Then those concepts maybe aren't such a good idea after all. Storing
lots of compressible data in an uncompressed fashion isn't an all that
common usecase. I most certainly don't want postgres to optimize for
blank padded data, especially if it can hurt other scenarios. Just not
enough benefit.
That said, I actually have relatively high hopes for compressing full
page writes. There often enough is lot of repetitiveness between rows on
the same page that it should be useful outside of such strange
scenarios. But maybe pglz is just not a good fit for this, it really
isn't a very good algorithm in this day and aage.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Fri, Oct 11, 2013 at 10:36 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2013-10-11 09:22:50 +0530, Amit Kapila wrote:
>> I think it will be difficult to prove by using any compression
>> algorithm, that it compresses in most of the scenario's.
>> In many cases it can so happen that the WAL will also not be reduced
>> and tps can also come down if the data is non-compressible, because
>> any compression algorithm will have to try to compress the data and it
>> will burn some cpu for that, which inturn will reduce tps.
>
> Then those concepts maybe aren't such a good idea after all. Storing
> lots of compressible data in an uncompressed fashion isn't an all that
> common usecase. I most certainly don't want postgres to optimize for
> blank padded data, especially if it can hurt other scenarios. Just not
> enough benefit.
> That said, I actually have relatively high hopes for compressing full
> page writes. There often enough is lot of repetitiveness between rows on
> the same page that it should be useful outside of such strange
> scenarios. But maybe pglz is just not a good fit for this, it really
> isn't a very good algorithm in this day and aage.

Do you think that if WAL reduction or performance with other
compression algorithm (for ex. snappy) is better, then chances of
getting the new compression algorithm in postresql will be more?
Wouldn't it be okay, if we have GUC to enable it and have pluggable
api for calling compression method, with this we can even include
other compression algorithm's if they proved to be good and reduce the
dependency of this patch on inclusion of new compression methods in
postgresql?

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On 11/10/13 19:06, Andres Freund wrote:
> On 2013-10-11 09:22:50 +0530, Amit Kapila wrote:
>> I think it will be difficult to prove by using any compression
>> algorithm, that it compresses in most of the scenario's.
>> In many cases it can so happen that the WAL will also not be reduced
>> and tps can also come down if the data is non-compressible, because
>> any compression algorithm will have to try to compress the data and it
>> will burn some cpu for that, which inturn will reduce tps.
> Then those concepts maybe aren't such a good idea after all. Storing
> lots of compressible data in an uncompressed fashion isn't an all that
> common usecase. I most certainly don't want postgres to optimize for
> blank padded data, especially if it can hurt other scenarios. Just not
> enough benefit.
> That said, I actually have relatively high hopes for compressing full
> page writes. There often enough is lot of repetitiveness between rows on
> the same page that it should be useful outside of such strange
> scenarios. But maybe pglz is just not a good fit for this, it really
> isn't a very good algorithm in this day and aage.
>
Hm,. There is a clear benefit for compressible data and clearly
no benefit from incompressible data..

how about letting autovacuum "taste" the compressibillity of
pages on per relation/index basis and set a flag that triggers
this functionality where it provides a benefit?

not hugely more magical than figuring out wether the data ends up
in the heap or in a toast table as it is now.

--
Jesper

(2013/10/13 0:14), Amit Kapila wrote:
> On Fri, Oct 11, 2013 at 10:36 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>> But maybe pglz is just not a good fit for this, it really
>> isn't a very good algorithm in this day and aage.
+1. This compression algorithm is needed more faster than pglz which is like
general compression algorithm, to avoid the CPU bottle-neck. I think pglz doesn't
have good performance, and it is like fossil compression algorithm. So we need to
change latest compression algorithm for more better future.

> Do you think that if WAL reduction or performance with other
> compression algorithm (for ex. snappy) is better, then chances of
> getting the new compression algorithm in postresql will be more?
Latest compression algorithms papers(also snappy) have indecated. I think it is
enough to select algorithm. It may be also good work in postgres.

Regards,
--
Mitsumasa KONDO
NTT Open Source Software Center

On Tue, Oct 15, 2013 at 6:30 AM, KONDO Mitsumasa
<kondo(dot)mitsumasa(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> (2013/10/13 0:14), Amit Kapila wrote:
>>
>> On Fri, Oct 11, 2013 at 10:36 PM, Andres Freund <andres(at)2ndquadrant(dot)com>
>> wrote:
>>>
>>> But maybe pglz is just not a good fit for this, it really
>>> isn't a very good algorithm in this day and aage.
>
> +1. This compression algorithm is needed more faster than pglz which is like
> general compression algorithm, to avoid the CPU bottle-neck. I think pglz
> doesn't have good performance, and it is like fossil compression algorithm.
> So we need to change latest compression algorithm for more better future.
>
>
>> Do you think that if WAL reduction or performance with other
>> compression algorithm (for ex. snappy) is better, then chances of
>> getting the new compression algorithm in postresql will be more?
>
> Latest compression algorithms papers(also snappy) have indecated. I think it
> is enough to select algorithm. It may be also good work in postgres.

Snappy is good mainly for un-compressible data, see the link below:
http://www.postgresql.org/message-id/CAAZKuFZCOCHsswQM60ioDO_hk12tA7OG3YcJA8v=4YebMOA-wA@mail.gmail.com

I think it is bit difficult to prove that any one algorithm is best
for all kind of loads.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

(2013/10/15 13:33), Amit Kapila wrote:
> Snappy is good mainly for un-compressible data, see the link below:
> http://www.postgresql.org/message-id/CAAZKuFZCOCHsswQM60ioDO_hk12tA7OG3YcJA8v=4YebMOA-wA@mail.gmail.com
This result was gotten in ARM architecture, it is not general CPU.
Please see detail document.
http://www.reddit.com/r/programming/comments/1aim6s/lz4_extremely_fast_compression_algorithm/c8y0ew9

I found compression algorithm test in HBase. I don't read detail, but it
indicates snnapy algorithm gets best performance.
http://blog.erdemagaoglu.com/post/4605524309/lzo-vs-snappy-vs-lzf-vs-zlib-a-comparison-of

In fact, most of modern NoSQL storages use snappy. Because it has good
performance and good licence(BSD license).

> I think it is bit difficult to prove that any one algorithm is best
> for all kind of loads.
I think it is necessary to make best efforts in community than I do the best
choice with strict test.

Regards,
--
Mitsumasa KONDO
NTT Open Source Software Center

On Tue, Oct 15, 2013 at 03:11:22PM +0900, KONDO Mitsumasa wrote:
> (2013/10/15 13:33), Amit Kapila wrote:
> >Snappy is good mainly for un-compressible data, see the link below:
> >http://www.postgresql.org/message-id/CAAZKuFZCOCHsswQM60ioDO_hk12tA7OG3YcJA8v=4YebMOA-wA@mail.gmail.com
> This result was gotten in ARM architecture, it is not general CPU.
> Please see detail document.
> http://www.reddit.com/r/programming/comments/1aim6s/lz4_extremely_fast_compression_algorithm/c8y0ew9
>
> I found compression algorithm test in HBase. I don't read detail,
> but it indicates snnapy algorithm gets best performance.
> http://blog.erdemagaoglu.com/post/4605524309/lzo-vs-snappy-vs-lzf-vs-zlib-a-comparison-of
>
> In fact, most of modern NoSQL storages use snappy. Because it has
> good performance and good licence(BSD license).
>
> >I think it is bit difficult to prove that any one algorithm is best
> >for all kind of loads.
> I think it is necessary to make best efforts in community than I do
> the best choice with strict test.
>
> Regards,
> --
> Mitsumasa KONDO
> NTT Open Source Software Center
>

Google's lz4 is also a very nice algorithm with 33% better compression
performance than snappy and 2X the decompression performance in some
benchmarks also with a bsd license:

https://code.google.com/p/lz4/

Regards,
Ken

(2013/10/15 22:01), ktm(at)rice(dot)edu wrote:
> Google's lz4 is also a very nice algorithm with 33% better compression
> performance than snappy and 2X the decompression performance in some
> benchmarks also with a bsd license:
>
> https://code.google.com/p/lz4/
If we judge only performance, we will select lz4. However, we should think
another important factor which is software robustness, achievement, bug
fix history, and etc... If we see unknown bugs, can we fix it or improve
algorithm? It seems very difficult, because we only use it and don't
understand algorihtms. Therefore, I think that we had better to select
robust and having more user software.

Regards,
--
Mitsumasa KONDO
NTT Open Source Software

On Wed, Oct 16, 2013 at 01:42:34PM +0900, KONDO Mitsumasa wrote:
> (2013/10/15 22:01), ktm(at)rice(dot)edu wrote:
> >Google's lz4 is also a very nice algorithm with 33% better compression
> >performance than snappy and 2X the decompression performance in some
> >benchmarks also with a bsd license:
> >
> >https://code.google.com/p/lz4/
> If we judge only performance, we will select lz4. However, we should think
> another important factor which is software robustness, achievement, bug
> fix history, and etc... If we see unknown bugs, can we fix it or improve
> algorithm? It seems very difficult, because we only use it and don't
> understand algorihtms. Therefore, I think that we had better to select
> robust and having more user software.
>
> Regards,
> --
> Mitsumasa KONDO
> NTT Open Source Software
>
Hi,

Those are all very good points. lz4 however is being used by Hadoop. It
is implemented natively in the Linux 3.11 kernel and the BSD version of
the ZFS filesystem supports the lz4 algorithm for on-the-fly compression.
With more and more CPU cores available in modern system, using an
algorithm with very fast decompression speeds can make storing data, even
in memory, in a compressed form can reduce space requirements in exchange
for a higher CPU cycle cost. The ability to make those sorts of trade-offs
can really benefit from a plug-able compression algorithm interface.

Regards,
Ken

Hi,

Sorry for my reply late...

(2013/10/11 2:32), Fujii Masao wrote:
> Could you let me know how much WAL records were generated
> during each benchmark?
It was not seen difference hardly about WAL in DBT-2 benchmark. It was because
largest tuples are filled in random character which is difficult to compress, I
survey it.

So I test two pattern data. One is original data which is hard to compress data.
Second is little bit changing data which are easy to compress data. Specifically,
I substitute zero padding tuple for random character tuple.
Record size is same in original test data, I changed only character fo record.
Sample changed record is here.

* Original record (item table)
> 1 9830 W+ùMî/aGhÞVJ;t+Pöþm5v2î. 82.62 Tî%N#ROò|?ö;[_îë~!YäHPÜï[S!JV58Ü#;+$cPì=dãNò;=Þô5
> 2 1492 VIKëyC..UCçWSèQð2?&s÷Jf 95.78 >ýoCj'nîHR`i]cøuDH&-wì4èè}{39ámLß2mC712Tao÷
> 3 4485 oJ)kLvP^_:91BOïé 32.00 ð<èüJ÷RÝ_Jze+?é4Ü7ä-r=DÝK\\$;Fsà8ál5

* Changed sample record (item table)
> 1 9830 000000000000000000000000 95.77 00000000000000000000000000000000000000000
> 2 764 00000000000000 47.92 00000000000000000000000000000000000000000000000000
> 3 4893 000000000000000000000 15.90 00000000000000000000000000000000000

* DBT-2 Result

@Werehouse = 340
| NOTPM | 90%tile | Average | S.Deviation
------------------------+-----------+-------------+---------+-------------
no-patched | 3319.02 | 13.606648 | 7.589 | 8.428
patched | 3341.25 | 20.132364 | 7.471 | 10.458
patched-testdata_changed| 3738.07 | 20.493533 | 3.795 | 10.003

Compression patch gets higher performance than no-patch in easy to compress test
data. It is because compression patch make archive WAL more small size, in
result, waste file cache is less than no-patch. Therefore, it was inflected
file-cache more effectively.

However, test in hard to compress test data have little bit lessor performance
than no-patch. I think it is compression overhead in pglz.

> I think that this benchmark result clearly means that the patch
> has only limited effects in the reduction of WAL volume and
> the performance improvement unless the database contains
> highly-compressible data like pgbench_accounts.
Your expectation is right. I think that low CPU cost and high compression
algorithm make your patch more better and better performance, too.

> filler. But if
> we can use other compression algorithm, maybe we can reduce
> WAL volume very much.
Yes, Please!

> I'm not sure what algorithm is good for WAL compression, though.
Community member think Snappy or lz4 is better. You'd better to select one,
or test two algorithms.

> It might be better to introduce the hook for compression of FPW
> so that users can freely use their compression module, rather
> than just using pglz_compress(). Thought?
In my memory, Andres Freund developed like this patch. Did it commit or
developing now? I have thought this idea is very good.

Regards,
--
Mitsumasa KONDO
NTT Open Source Software Center

On Tue, Oct 15, 2013 at 11:41 AM, KONDO Mitsumasa
<kondo(dot)mitsumasa(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> (2013/10/15 13:33), Amit Kapila wrote:
>>
>> Snappy is good mainly for un-compressible data, see the link below:
>>
>> http://www.postgresql.org/message-id/CAAZKuFZCOCHsswQM60ioDO_hk12tA7OG3YcJA8v=4YebMOA-wA@mail.gmail.com
>
> This result was gotten in ARM architecture, it is not general CPU.
> Please see detail document.
> http://www.reddit.com/r/programming/comments/1aim6s/lz4_extremely_fast_compression_algorithm/c8y0ew9

I think in general also snappy is mostly preferred for it's low CPU
usage not for compression, but overall my vote is also for snappy.

> I found compression algorithm test in HBase. I don't read detail, but it
> indicates snnapy algorithm gets best performance.
> http://blog.erdemagaoglu.com/post/4605524309/lzo-vs-snappy-vs-lzf-vs-zlib-a-comparison-of

The dataset used for performance is quite different from the data
which we are talking about here (WAL).
"These are the scores for a data which consist of 700kB rows, each
containing a binary image data. They probably won’t apply to things
like numeric or text data."

> In fact, most of modern NoSQL storages use snappy. Because it has good
> performance and good licence(BSD license).
>
>
>> I think it is bit difficult to prove that any one algorithm is best
>> for all kind of loads.
>
> I think it is necessary to make best efforts in community than I do the best
> choice with strict test.

Sure, it is good to make effort to select the best algorithm, but if
you are combining this patch with inclusion of new compression
algorithm in PG, it can only make the patch to take much longer time.

In general, my thinking is that we should prefer compression to reduce
IO (WAL volume), because reducing WAL volume has other benefits as
well like sending it to subscriber nodes. I think it will help cases
where due to less n/w bandwidth, the disk allocated for WAL becomes
full due to high traffic on master and then users need some
alternative methods to handle such situations.

I think many users would like to use a method which can reduce WAL
volume and the users which don't find it enough useful in their
environments due to decrease in TPS or not significant reduction in
WAL have the option to disable it.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

(2013/10/19 14:58), Amit Kapila wrote:
> On Tue, Oct 15, 2013 at 11:41 AM, KONDO Mitsumasa
> <kondo(dot)mitsumasa(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> I think in general also snappy is mostly preferred for it's low CPU
> usage not for compression, but overall my vote is also for snappy.
I think low CPU usage is the best important factor in WAL compression.
It is because WAL write is sequencial write, so few compression ratio improvement
cannot change PostgreSQL's performance, and furthermore raid card with writeback
feature. Furthermore PG executes programs by single proccess, high CPU usage
compression algorithm will cause lessor performance.

>> I found compression algorithm test in HBase. I don't read detail, but it
>> indicates snnapy algorithm gets best performance.
>>
http://blog.erdemagaoglu.com/post/4605524309/lzo-vs-snappy-vs-lzf-vs-zlib-a-comparison-of
>
> The dataset used for performance is quite different from the data
> which we are talking about here (WAL).
> "These are the scores for a data which consist of 700kB rows, each
> containing a binary image data. They probably won’t apply to things
> like numeric or text data."
Yes, you are right. We need testing about compression algorithm in WAL write.

>> I think it is necessary to make best efforts in community than I do the best
>> choice with strict test.
>
> Sure, it is good to make effort to select the best algorithm, but if
> you are combining this patch with inclusion of new compression
> algorithm in PG, it can only make the patch to take much longer time.
I think if our direction is specifically decided, it is easy to make the patch.
Complession patch's direction isn't still become clear, it will be a troublesome
patch which is like sync-rep patch.

> In general, my thinking is that we should prefer compression to reduce
> IO (WAL volume), because reducing WAL volume has other benefits as
> well like sending it to subscriber nodes. I think it will help cases
> where due to less n/w bandwidth, the disk allocated for WAL becomes
> full due to high traffic on master and then users need some
> alternative methods to handle such situations.
Do you talk about archiving WAL file? It can easy to reduce volume that we set
and add compression command with copy command at archive_command.

> I think many users would like to use a method which can reduce WAL
> volume and the users which don't find it enough useful in their
> environments due to decrease in TPS or not significant reduction in
> WAL have the option to disable it.
I favor to select compression algorithm for higher performance. If we need to
compress WAL file more, in spite of lessor performance, we can change archive
copy command with high compression algorithm and add documents that how to
compress archive WAL files at archive_command. Does it wrong? In actual, many of
NoSQLs use snappy for purpose of higher performance.

Regards,
--
Mitsumasa KONDO
NTT Open Source Software Center

On Mon, Oct 21, 2013 at 4:40 PM, KONDO Mitsumasa
<kondo(dot)mitsumasa(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> (2013/10/19 14:58), Amit Kapila wrote:
>> On Tue, Oct 15, 2013 at 11:41 AM, KONDO Mitsumasa
>> <kondo(dot)mitsumasa(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>> In general, my thinking is that we should prefer compression to reduce
>> IO (WAL volume), because reducing WAL volume has other benefits as
>> well like sending it to subscriber nodes. I think it will help cases
>> where due to less n/w bandwidth, the disk allocated for WAL becomes
>> full due to high traffic on master and then users need some
>> alternative methods to handle such situations.
> Do you talk about archiving WAL file?

One of the points what I am talking about is sending data over
network to subscriber nodes for streaming replication and another is
WAL in pg_xlog. Both scenario's get benefited if there is is WAL
volume.

> It can easy to reduce volume that we
> set and add compression command with copy command at archive_command.

Okay.

>> I think many users would like to use a method which can reduce WAL
>> volume and the users which don't find it enough useful in their
>> environments due to decrease in TPS or not significant reduction in
>> WAL have the option to disable it.
> I favor to select compression algorithm for higher performance. If we need
> to compress WAL file more, in spite of lessor performance, we can change
> archive copy command with high compression algorithm and add documents that
> how to compress archive WAL files at archive_command. Does it wrong?

No, it is not wrong, but there are scenario's as mentioned above
where less WAL volume can be beneficial.

> In
> actual, many of NoSQLs use snappy for purpose of higher performance.

Okay, you can also check the results with snappy algorithm, but don't
just rely completely on snappy for this patch, you might want to think
of another alternative for this patch.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On Tue, Oct 22, 2013 at 12:47 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Mon, Oct 21, 2013 at 4:40 PM, KONDO Mitsumasa
> <kondo(dot)mitsumasa(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>> (2013/10/19 14:58), Amit Kapila wrote:
>>> On Tue, Oct 15, 2013 at 11:41 AM, KONDO Mitsumasa
>>> <kondo(dot)mitsumasa(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>>> In general, my thinking is that we should prefer compression to reduce
>>> IO (WAL volume), because reducing WAL volume has other benefits as
>>> well like sending it to subscriber nodes. I think it will help cases
>>> where due to less n/w bandwidth, the disk allocated for WAL becomes
>>> full due to high traffic on master and then users need some
>>> alternative methods to handle such situations.
>> Do you talk about archiving WAL file?
>
> One of the points what I am talking about is sending data over
> network to subscriber nodes for streaming replication and another is
> WAL in pg_xlog. Both scenario's get benefited if there is is WAL
> volume.
>
>> It can easy to reduce volume that we
>> set and add compression command with copy command at archive_command.
>
> Okay.
>
>>> I think many users would like to use a method which can reduce WAL
>>> volume and the users which don't find it enough useful in their
>>> environments due to decrease in TPS or not significant reduction in
>>> WAL have the option to disable it.
>> I favor to select compression algorithm for higher performance. If we need
>> to compress WAL file more, in spite of lessor performance, we can change
>> archive copy command with high compression algorithm and add documents that
>> how to compress archive WAL files at archive_command. Does it wrong?
>
> No, it is not wrong, but there are scenario's as mentioned above
> where less WAL volume can be beneficial.
>
>> In
>> actual, many of NoSQLs use snappy for purpose of higher performance.
>
> Okay, you can also check the results with snappy algorithm, but don't
> just rely completely on snappy for this patch, you might want to think
> of another alternative for this patch.

So, our consensus is to introduce the hooks for FPW compression so that
users can freely select their own best compression algorithm?
Also, probably we need to implement at least one compression contrib module
using that hook, maybe it's based on pglz or snappy.

Regards,

--
Fujii Masao

On Tue, Oct 22, 2013 at 9:22 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> On Tue, Oct 22, 2013 at 12:47 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> On Mon, Oct 21, 2013 at 4:40 PM, KONDO Mitsumasa
>> <kondo(dot)mitsumasa(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>>> (2013/10/19 14:58), Amit Kapila wrote:
>>>> On Tue, Oct 15, 2013 at 11:41 AM, KONDO Mitsumasa
>>>> <kondo(dot)mitsumasa(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>>
>>> In
>>> actual, many of NoSQLs use snappy for purpose of higher performance.
>>
>> Okay, you can also check the results with snappy algorithm, but don't
>> just rely completely on snappy for this patch, you might want to think
>> of another alternative for this patch.
>
> So, our consensus is to introduce the hooks for FPW compression so that
> users can freely select their own best compression algorithm?

We can also provide GUC for whether to enable WAL compression, which
I think you are also planing to include based on some previous e-mails
in this thread.

You can consider my vote for this idea. However I think we should wait
to see if anyone else have objection to this idea.

> Also, probably we need to implement at least one compression contrib module
> using that hook, maybe it's based on pglz or snappy.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On 2013-10-22 12:52:09 +0900, Fujii Masao wrote:
> So, our consensus is to introduce the hooks for FPW compression so that
> users can freely select their own best compression algorithm?

No, I don't think that's concensus yet. If you want to make it
configurable on that level you need to have:
1) compression format signature on fpws
2) mapping between identifiers for compression formats and the libraries implementing them.

Otherwise you can only change the configuration at initdb time...

> Also, probably we need to implement at least one compression contrib module
> using that hook, maybe it's based on pglz or snappy.

From my tests for toast compression I'd suggest starting with lz4.

I'd suggest starting by publishing test results with a more modern
compression formats, but without hacks like increasing padding.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

(2013/10/22 12:52), Fujii Masao wrote:
> On Tue, Oct 22, 2013 at 12:47 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> On Mon, Oct 21, 2013 at 4:40 PM, KONDO Mitsumasa
>> <kondo(dot)mitsumasa(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>>> (2013/10/19 14:58), Amit Kapila wrote:
>>>> On Tue, Oct 15, 2013 at 11:41 AM, KONDO Mitsumasa
>>>> <kondo(dot)mitsumasa(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>>>> In general, my thinking is that we should prefer compression to reduce
>>>> IO (WAL volume), because reducing WAL volume has other benefits as
>>>> well like sending it to subscriber nodes. I think it will help cases
>>>> where due to less n/w bandwidth, the disk allocated for WAL becomes
>>>> full due to high traffic on master and then users need some
>>>> alternative methods to handle such situations.
>>> Do you talk about archiving WAL file?
>>
>> One of the points what I am talking about is sending data over
>> network to subscriber nodes for streaming replication and another is
>> WAL in pg_xlog. Both scenario's get benefited if there is is WAL
>> volume.
>>
>>> It can easy to reduce volume that we
>>> set and add compression command with copy command at archive_command.
>>
>> Okay.
>>
>>>> I think many users would like to use a method which can reduce WAL
>>>> volume and the users which don't find it enough useful in their
>>>> environments due to decrease in TPS or not significant reduction in
>>>> WAL have the option to disable it.
>>> I favor to select compression algorithm for higher performance. If we need
>>> to compress WAL file more, in spite of lessor performance, we can change
>>> archive copy command with high compression algorithm and add documents that
>>> how to compress archive WAL files at archive_command. Does it wrong?
>>
>> No, it is not wrong, but there are scenario's as mentioned above
>> where less WAL volume can be beneficial.
>>
>>> In
>>> actual, many of NoSQLs use snappy for purpose of higher performance.
>>
>> Okay, you can also check the results with snappy algorithm, but don't
>> just rely completely on snappy for this patch, you might want to think
>> of another alternative for this patch.
>
> So, our consensus is to introduce the hooks for FPW compression so that
> users can freely select their own best compression algorithm?
Yes, it will be also good for future improvement. But I think WAL compression for
disaster recovery system should be need in walsender and walreceiver proccess,
and it is propety architecture for DR system. Higher compression ratio with high
CPU usage algorithm in FPW might affect bad for perfomance in master server. If
we can set compression algorithm in walsender and walreciever, performance is
same as before or better, and WAL send performance will be better.

Regards,
--
Mitsumasa KONDO
NTT Open Source Software Center

On Wed, Oct 23, 2013 at 7:05 AM, KONDO Mitsumasa
<kondo(dot)mitsumasa(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> (2013/10/22 12:52), Fujii Masao wrote:
>>
>> On Tue, Oct 22, 2013 at 12:47 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
>> wrote:
>>>
>>> On Mon, Oct 21, 2013 at 4:40 PM, KONDO Mitsumasa
>>> <kondo(dot)mitsumasa(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>>>>
>>>> (2013/10/19 14:58), Amit Kapila wrote:
>>>>>
>>>>> On Tue, Oct 15, 2013 at 11:41 AM, KONDO Mitsumasa
>>>>> <kondo(dot)mitsumasa(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>>>> In
>>>> actual, many of NoSQLs use snappy for purpose of higher performance.
>>>
>>>
>>> Okay, you can also check the results with snappy algorithm, but don't
>>> just rely completely on snappy for this patch, you might want to think
>>> of another alternative for this patch.
>>
>>
>> So, our consensus is to introduce the hooks for FPW compression so that
>> users can freely select their own best compression algorithm?
>
> Yes, it will be also good for future improvement. But I think WAL
> compression for disaster recovery system should be need in walsender and
> walreceiver proccess, and it is propety architecture for DR system. Higher
> compression ratio with high CPU usage algorithm in FPW might affect bad for
> perfomance in master server.

This is true, thats why there is a discussion for pluggable API for
compression of WAL, we should try to choose best algorithm from the
available choices. Even after that I am not sure it works same for all
kind of loads, so user will have option to completely disable it as
well.

> If we can set compression algorithm in
> walsender and walreciever, performance is same as before or better, and WAL
> send performance will be better.

Do you mean to say that walsender should compress the data before
sending and then walreceiver will decompress it, if yes then won't it
add extra overhead on standby, or do you think as walreceiver has to
read less data from socket, so it will compensate for it. I think may
be we should consider this if the test results are good, but lets not
try to do this until the current patch proves that such mechanism is
good for WAL compression.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On Mon, Oct 21, 2013 at 11:52 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> So, our consensus is to introduce the hooks for FPW compression so that
> users can freely select their own best compression algorithm?
> Also, probably we need to implement at least one compression contrib module
> using that hook, maybe it's based on pglz or snappy.

I don't favor making this pluggable. I think we should pick snappy or
lz4 (or something else), put it in the tree, and use it.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Mon, Oct 21, 2013 at 11:52 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> So, our consensus is to introduce the hooks for FPW compression so that
>> users can freely select their own best compression algorithm?
>> Also, probably we need to implement at least one compression contrib module
>> using that hook, maybe it's based on pglz or snappy.

> I don't favor making this pluggable. I think we should pick snappy or
> lz4 (or something else), put it in the tree, and use it.

I agree. Hooks in this area are going to be a constant source of
headaches, vastly outweighing any possible benefit.

regards, tom lane

On Thu, Oct 24, 2013 at 11:07:38AM -0400, Robert Haas wrote:
> On Mon, Oct 21, 2013 at 11:52 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> > So, our consensus is to introduce the hooks for FPW compression so that
> > users can freely select their own best compression algorithm?
> > Also, probably we need to implement at least one compression contrib module
> > using that hook, maybe it's based on pglz or snappy.
>
> I don't favor making this pluggable. I think we should pick snappy or
> lz4 (or something else), put it in the tree, and use it.
>
Hi,

My vote would be for lz4 since it has faster single thread compression
and decompression speeds with the decompression speed being almost 2X
snappy's decompression speed. The both are BSD licensed so that is not
an issue. The base code for lz4 is c and it is c++ for snappy. There
is also a HC (high-compression) varient for lz4 that pushes its compression
rate to about the same as zlib (-1) which uses the same decompressor which
can provide data even faster due to better compression. Some more real
world tests would be useful, which is really where being pluggable would
help.

Regards,
Ken

On Thu, Oct 24, 2013 at 11:40 AM, ktm(at)rice(dot)edu <ktm(at)rice(dot)edu> wrote:
> On Thu, Oct 24, 2013 at 11:07:38AM -0400, Robert Haas wrote:
>> On Mon, Oct 21, 2013 at 11:52 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> > So, our consensus is to introduce the hooks for FPW compression so that
>> > users can freely select their own best compression algorithm?
>> > Also, probably we need to implement at least one compression contrib module
>> > using that hook, maybe it's based on pglz or snappy.
>>
>> I don't favor making this pluggable. I think we should pick snappy or
>> lz4 (or something else), put it in the tree, and use it.
>>
> Hi,
>
> My vote would be for lz4 since it has faster single thread compression
> and decompression speeds with the decompression speed being almost 2X
> snappy's decompression speed. The both are BSD licensed so that is not
> an issue. The base code for lz4 is c and it is c++ for snappy. There
> is also a HC (high-compression) varient for lz4 that pushes its compression
> rate to about the same as zlib (-1) which uses the same decompressor which
> can provide data even faster due to better compression. Some more real
> world tests would be useful, which is really where being pluggable would
> help.

Well, it's probably a good idea for us to test, during the development
cycle, which algorithm works better for WAL compression, and then use
that one. Once we make that decision, I don't see that there are many
circumstances in which a user would care to override it. Now if we
find that there ARE reasons for users to prefer different algorithms
in different situations, that would be a good reason to make it
configurable (or even pluggable). But if we find that no such reasons
exist, then we're better off avoiding burdening users with the need to
configure a setting that has only one sensible value.

It seems fairly clear from previous discussions on this mailing list
that snappy and lz4 are the top contenders for the position of
"compression algorithm favored by PostgreSQL". I am wondering,
though, whether it wouldn't be better to add support for both - say we
added both to libpgcommon, and perhaps we could consider moving pglz
there as well. That would allow easy access to all of those
algorithms from both front-end and backend-code. If we can make the
APIs parallel, it should very simple to modify any code we add now to
use a different algorithm than the one initially chosen if in the
future we add algorithms to or remove algorithms from the list, or if
one algorithm is shown to outperform another in some particular
context. I think we'll do well to isolate the question of adding
support for these algorithms form the current patch or any other
particular patch that may be on the table, and FWIW, I think having
two leading contenders and adding support for both may have a variety
of advantages over crowning a single victor.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Thu, Oct 24, 2013 at 12:22:59PM -0400, Robert Haas wrote:
> On Thu, Oct 24, 2013 at 11:40 AM, ktm(at)rice(dot)edu <ktm(at)rice(dot)edu> wrote:
> > On Thu, Oct 24, 2013 at 11:07:38AM -0400, Robert Haas wrote:
> >> On Mon, Oct 21, 2013 at 11:52 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> >> > So, our consensus is to introduce the hooks for FPW compression so that
> >> > users can freely select their own best compression algorithm?
> >> > Also, probably we need to implement at least one compression contrib module
> >> > using that hook, maybe it's based on pglz or snappy.
> >>
> >> I don't favor making this pluggable. I think we should pick snappy or
> >> lz4 (or something else), put it in the tree, and use it.
> >>
> > Hi,
> >
> > My vote would be for lz4 since it has faster single thread compression
> > and decompression speeds with the decompression speed being almost 2X
> > snappy's decompression speed. The both are BSD licensed so that is not
> > an issue. The base code for lz4 is c and it is c++ for snappy. There
> > is also a HC (high-compression) varient for lz4 that pushes its compression
> > rate to about the same as zlib (-1) which uses the same decompressor which
> > can provide data even faster due to better compression. Some more real
> > world tests would be useful, which is really where being pluggable would
> > help.
>
> Well, it's probably a good idea for us to test, during the development
> cycle, which algorithm works better for WAL compression, and then use
> that one. Once we make that decision, I don't see that there are many
> circumstances in which a user would care to override it. Now if we
> find that there ARE reasons for users to prefer different algorithms
> in different situations, that would be a good reason to make it
> configurable (or even pluggable). But if we find that no such reasons
> exist, then we're better off avoiding burdening users with the need to
> configure a setting that has only one sensible value.
>
> It seems fairly clear from previous discussions on this mailing list
> that snappy and lz4 are the top contenders for the position of
> "compression algorithm favored by PostgreSQL". I am wondering,
> though, whether it wouldn't be better to add support for both - say we
> added both to libpgcommon, and perhaps we could consider moving pglz
> there as well. That would allow easy access to all of those
> algorithms from both front-end and backend-code. If we can make the
> APIs parallel, it should very simple to modify any code we add now to
> use a different algorithm than the one initially chosen if in the
> future we add algorithms to or remove algorithms from the list, or if
> one algorithm is shown to outperform another in some particular
> context. I think we'll do well to isolate the question of adding
> support for these algorithms form the current patch or any other
> particular patch that may be on the table, and FWIW, I think having
> two leading contenders and adding support for both may have a variety
> of advantages over crowning a single victor.
>
+++1

Ken

On Thu, Oct 24, 2013 at 8:37 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Mon, Oct 21, 2013 at 11:52 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> So, our consensus is to introduce the hooks for FPW compression so that
>> users can freely select their own best compression algorithm?
>> Also, probably we need to implement at least one compression contrib module
>> using that hook, maybe it's based on pglz or snappy.
>
> I don't favor making this pluggable. I think we should pick snappy or
> lz4 (or something else), put it in the tree, and use it.

The reason why the discussion went towards making it pluggable (or at
least what made me to think like that) was because of below reasons:
a. what somebody needs to do to make snappy or lz4 in the tree, is it
only performance/compression data for some of the scenario's or some
other legal
stuff as well, if it is only performance/compression then what
will be the scenario's (is pgbench sufficient?).
b. there can be cases where one or the other algorithm can be better
or not doing compression is better. For example in one of the other
patches where
we were trying to achieve WAL reduction in Update operation
(http://www.postgresql.org/message-id/8977CB36860C5843884E0A18D8747B036B9A4B04@szxeml558-mbs.china.huawei.com),
Heikki has came up with a test (where data is not much
compressible), in such a case, the observation was that LZ was better
than native
compression method used in that patch and Snappy was better than
LZ and not doing compression could be considered preferable in such a
scenario because all the algorithm's were reducing TPS for that case.

Now I think it is certainly better if we could choose one of the
algorithms (snappy or lz4) and test them for most used scenario's for
compression and performance and call it done, but I think giving at
least an option to user to make compression altogether off should be
still considered.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On Fri, Oct 11, 2013 at 12:30:41PM +0900, Fujii Masao wrote:
> > Sure. To be honest, when I received the same request from Andres,
> > I did that benchmark. But unfortunately because of machine trouble,
> > I could not report it, yet. Will do that again.
>
> Here is the benchmark result:
>
> * Result
> [tps]
> 1317.306391 (full_page_writes = on)
> 1628.407752 (compress)
>
> [the amount of WAL generated during running pgbench]
> 1319 MB (on)
> 326 MB (compress)
>
> [time required to replay WAL generated during running pgbench]
> 19s (on)
> 2013-10-11 12:05:09 JST LOG: redo starts at F/F1000028
> 2013-10-11 12:05:28 JST LOG: redo done at 10/446B7BF0
>
> 12s (on)
> 2013-10-11 12:06:22 JST LOG: redo starts at F/F1000028
> 2013-10-11 12:06:34 JST LOG: redo done at 10/446B7BF0
>
> 12s (on)
> 2013-10-11 12:07:19 JST LOG: redo starts at F/F1000028
> 2013-10-11 12:07:31 JST LOG: redo done at 10/446B7BF0
>
> 8s (compress)
> 2013-10-11 12:17:36 JST LOG: redo starts at 10/50000028
> 2013-10-11 12:17:44 JST LOG: redo done at 10/655AE478
>
> 8s (compress)
> 2013-10-11 12:18:26 JST LOG: redo starts at 10/50000028
> 2013-10-11 12:18:34 JST LOG: redo done at 10/655AE478
>
> 8s (compress)
> 2013-10-11 12:19:07 JST LOG: redo starts at 10/50000028
> 2013-10-11 12:19:15 JST LOG: redo done at 10/655AE478

Fujii, are you still working on this? I sure hope so.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

On Sat, Feb 1, 2014 at 10:22 AM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> On Fri, Oct 11, 2013 at 12:30:41PM +0900, Fujii Masao wrote:
>> > Sure. To be honest, when I received the same request from Andres,
>> > I did that benchmark. But unfortunately because of machine trouble,
>> > I could not report it, yet. Will do that again.
>>
>> Here is the benchmark result:
>>
>> * Result
>> [tps]
>> 1317.306391 (full_page_writes = on)
>> 1628.407752 (compress)
>>
>> [the amount of WAL generated during running pgbench]
>> 1319 MB (on)
>> 326 MB (compress)
>>
>> [time required to replay WAL generated during running pgbench]
>> 19s (on)
>> 2013-10-11 12:05:09 JST LOG: redo starts at F/F1000028
>> 2013-10-11 12:05:28 JST LOG: redo done at 10/446B7BF0
>>
>> 12s (on)
>> 2013-10-11 12:06:22 JST LOG: redo starts at F/F1000028
>> 2013-10-11 12:06:34 JST LOG: redo done at 10/446B7BF0
>>
>> 12s (on)
>> 2013-10-11 12:07:19 JST LOG: redo starts at F/F1000028
>> 2013-10-11 12:07:31 JST LOG: redo done at 10/446B7BF0
>>
>> 8s (compress)
>> 2013-10-11 12:17:36 JST LOG: redo starts at 10/50000028
>> 2013-10-11 12:17:44 JST LOG: redo done at 10/655AE478
>>
>> 8s (compress)
>> 2013-10-11 12:18:26 JST LOG: redo starts at 10/50000028
>> 2013-10-11 12:18:34 JST LOG: redo done at 10/655AE478
>>
>> 8s (compress)
>> 2013-10-11 12:19:07 JST LOG: redo starts at 10/50000028
>> 2013-10-11 12:19:15 JST LOG: redo done at 10/655AE478
>
> Fujii, are you still working on this? I sure hope so.

Yes, but it's too late to implement and post new patch in this
development cycle of 9.4dev. I will propose that in next CF.

Regards,

--
Fujii Masao

Hello,
>Done. Attached is the updated version of the patch.
I was trying to check WAL reduction using this patch on latest available git
version of Postgres using JDBC runner with tpcc benchmark.

patching_problems.txt
<http://postgresql.1045698.n5.nabble.com/file/n5803482/patching_problems.txt>

I did resolve the patching conflicts and then compiled the source, removing
couple of compiler errors in process. But the server crashes in the compress
mode i.e. the moment any WAL is generated. Works fine in 'on' and 'off'
mode.
Clearly i must be resolving patch conflicts incorrectly as this patch
applied cleanly earlier. Is there a version of the source where i could
apply it the patch cleanly?

Thank you,
Sameer

--
View this message in context: http://postgresql.1045698.n5.nabble.com/Compression-of-full-page-writes-tp5769039p5803482.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

Sameer Thakur <samthakur74(at)gmail(dot)com> writes:
> I was trying to check WAL reduction using this patch on latest available git
> version of Postgres using JDBC runner with tpcc benchmark.
> patching_problems.txt
> <http://postgresql.1045698.n5.nabble.com/file/n5803482/patching_problems.txt>
> I did resolve the patching conflicts and then compiled the source, removing
> couple of compiler errors in process. But the server crashes in the compress
> mode i.e. the moment any WAL is generated. Works fine in 'on' and 'off'
> mode.
> Clearly i must be resolving patch conflicts incorrectly as this patch
> applied cleanly earlier. Is there a version of the source where i could
> apply it the patch cleanly?

If the patch used to work, it's a good bet that what broke it is the
recent pgindent run:
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=0a7832005792fa6dad171f9cadb8d587fe0dd800

It's going to need to be rebased past that, but doing so by hand would
be tedious, and evidently was error-prone too. If you've got pgindent
installed, you could consider applying the patch to the parent of
that commit, pgindent'ing the whole tree, and then diffing against
that commit to generate an updated patch.
See src/tools/pgindent/README for some build/usage notes about pgindent.

regards, tom lane

On Sat, May 10, 2014 at 8:33 PM, Sameer Thakur <samthakur74(at)gmail(dot)com> wrote:
> Hello,
>>Done. Attached is the updated version of the patch.
> I was trying to check WAL reduction using this patch on latest available git
> version of Postgres using JDBC runner with tpcc benchmark.
>
> patching_problems.txt
> <http://postgresql.1045698.n5.nabble.com/file/n5803482/patching_problems.txt>
>
> I did resolve the patching conflicts and then compiled the source, removing
> couple of compiler errors in process. But the server crashes in the compress
> mode i.e. the moment any WAL is generated. Works fine in 'on' and 'off'
> mode.

What kind of error did you get at the server crash? Assertion error? If yes,
it might be because of the conflict with
4a170ee9e0ebd7021cb1190fabd5b0cbe2effb8e.
This commit forbids palloc from being called within a critical section, but
the patch does that and then the assertion error happens. That's a bug of
the patch.

Regards,

--
Fujii Masao

Hello,

> What kind of error did you get at the server crash? Assertion error? If yes,
> it might be because of the conflict with
> 4a170ee9e0ebd7021cb1190fabd5b0cbe2effb8e.
> This commit forbids palloc from being called within a critical section, but
> the patch does that and then the assertion error happens. That's a bug of
> the patch.
seems to be that
STATEMENT: create table test (id integer);
TRAP: FailedAssertion("!(CritSectionCount == 0 ||
(CurrentMemoryContext) == ErrorContext || (MyAuxProcType ==
CheckpointerProcess))", File: "mcxt.c", Line: 670)
LOG: server process (PID 29721) was terminated by signal 6: Aborted
DETAIL: Failed process was running: drop table test;
LOG: terminating any other active server processes
WARNING: terminating connection because of crash of another server process
DETAIL: The postmaster has commanded this server process to roll back
the current transaction and exit, because another server process
exited abnormally and possibly corrupted shared memory.

How do i resolve this?
Thank you,
Sameer

On 30 August 2013 04:55, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:

> My idea is very simple, just compress FPW because FPW is
> a big part of WAL. I used pglz_compress() as a compression method,
> but you might think that other method is better. We can add
> something like FPW-compression-hook for that later. The patch
> adds new GUC parameter, but I'm thinking to merge it to full_page_writes
> parameter to avoid increasing the number of GUC. That is,
> I'm thinking to change full_page_writes so that it can accept new value
> 'compress'.

> * Result
> [tps]
> 1386.8 (compress_backup_block = off)
> 1627.7 (compress_backup_block = on)
>
> [the amount of WAL generated during running pgbench]
> 4302 MB (compress_backup_block = off)
> 1521 MB (compress_backup_block = on)

Compressing FPWs definitely makes sense for bulk actions.

I'm worried that the loss of performance occurs by greatly elongating
transaction response times immediately after a checkpoint, which were
already a problem. I'd be interested to look at the response time
curves there.

Maybe it makes sense to compress FPWs if we do, say, > N FPW writes in
a transaction. Just ideas.

I was thinking about this and about our previous thoughts about double
buffering. FPWs are made in foreground, so will always slow down
transaction rates. If we could move to double buffering we could avoid
FPWs altogether. Thoughts?

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Sun, May 11, 2014 at 7:30 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> On 30 August 2013 04:55, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>
>> My idea is very simple, just compress FPW because FPW is
>> a big part of WAL. I used pglz_compress() as a compression method,
>> but you might think that other method is better. We can add
>> something like FPW-compression-hook for that later. The patch
>> adds new GUC parameter, but I'm thinking to merge it to full_page_writes
>> parameter to avoid increasing the number of GUC. That is,
>> I'm thinking to change full_page_writes so that it can accept new value
>> 'compress'.
>
>> * Result
>> [tps]
>> 1386.8 (compress_backup_block = off)
>> 1627.7 (compress_backup_block = on)
>>
>> [the amount of WAL generated during running pgbench]
>> 4302 MB (compress_backup_block = off)
>> 1521 MB (compress_backup_block = on)
>
> Compressing FPWs definitely makes sense for bulk actions.
>
> I'm worried that the loss of performance occurs by greatly elongating
> transaction response times immediately after a checkpoint, which were
> already a problem. I'd be interested to look at the response time
> curves there.

Yep, I agree that we should check how the compression of FPW affects
the response time, especially just after checkpoint starts.

> I was thinking about this and about our previous thoughts about double
> buffering. FPWs are made in foreground, so will always slow down
> transaction rates. If we could move to double buffering we could avoid
> FPWs altogether. Thoughts?

If I understand the double buffering correctly, it would eliminate the need for
FPW. But I'm not sure how easy we can implement the double buffering.

Regards,

--
Fujii Masao

On Tue, May 13, 2014 at 3:33 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> On Sun, May 11, 2014 at 7:30 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>> On 30 August 2013 04:55, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>
>>> My idea is very simple, just compress FPW because FPW is
>>> a big part of WAL. I used pglz_compress() as a compression method,
>>> but you might think that other method is better. We can add
>>> something like FPW-compression-hook for that later. The patch
>>> adds new GUC parameter, but I'm thinking to merge it to full_page_writes
>>> parameter to avoid increasing the number of GUC. That is,
>>> I'm thinking to change full_page_writes so that it can accept new value
>>> 'compress'.
>>
>>> * Result
>>> [tps]
>>> 1386.8 (compress_backup_block = off)
>>> 1627.7 (compress_backup_block = on)
>>>
>>> [the amount of WAL generated during running pgbench]
>>> 4302 MB (compress_backup_block = off)
>>> 1521 MB (compress_backup_block = on)
>>
>> Compressing FPWs definitely makes sense for bulk actions.
>>
>> I'm worried that the loss of performance occurs by greatly elongating
>> transaction response times immediately after a checkpoint, which were
>> already a problem. I'd be interested to look at the response time
>> curves there.
>
> Yep, I agree that we should check how the compression of FPW affects
> the response time, especially just after checkpoint starts.
>
>> I was thinking about this and about our previous thoughts about double
>> buffering. FPWs are made in foreground, so will always slow down
>> transaction rates. If we could move to double buffering we could avoid
>> FPWs altogether. Thoughts?
>
> If I understand the double buffering correctly, it would eliminate the need for
> FPW. But I'm not sure how easy we can implement the double buffering.

There is already a patch on the double buffer write to eliminate the FPW.
But It has some performance problem because of CRC calculation for the
entire page.

http://www.postgresql.org/message-id/1962493974.656458.1327703514780.JavaMail.root@zimbra-prod-mbox-4.vmware.com

I think this patch can be further modified with a latest multi core
CRC calculation and can be used for testing.

Regards,
Hari Babu
Fujitsu Australia

Hello All,

0001-CompressBackupBlock_snappy_lz4_pglz extends patch on compression of
full page writes to include LZ4 and Snappy . Changes include making
"compress_backup_block" GUC from boolean to enum. Value of the GUC can be
OFF, pglz, snappy or lz4 which can be used to turn off compression or set
the desired compression algorithm.

0002-Support_snappy_lz4 adds support for LZ4 and Snappy in PostgreSQL. It
uses Andres’s patch for getting Makefiles working and has a few wrappers to
make the function calls to LZ4 and Snappy compression functions and handle
varlena datatypes.
Patch Courtesy: Pavan Deolasee

These patches serve as a way to test various compression algorithms. These
are WIP yet. They don’t support changing compression algorithms on standby .
Also, compress_backup_block GUC needs to be merged with full_page_writes.
The patch uses LZ4 high compression(HC) variant.
I have conducted initial tests which I would like to share and solicit
feedback

Tests use JDBC runner TPC-C benchmark to measure the amount of WAL
compression ,tps and response time in each of the scenarios viz .
Compression = OFF , pglz, LZ4 , snappy ,FPW=off

Server specifications:
Processors:Intel® Xeon ® Processor E5-2650 (2 GHz, 8C/16T, 20 MB) * 2 nos
RAM: 32GB
Disk : HDD 450GB 10K Hot Plug 2.5-inch SAS HDD * 8 nos
1 x 450 GB SAS HDD, 2.5-inch, 6Gb/s, 10,000 rpm

Benchmark:
Scale : 100
Command :java JR /home/postgres/jdbcrunner-1.2/scripts/tpcc.js -sleepTime
600,350,300,250,250
Warmup time : 1 sec
Measurement time : 900 sec
Number of tx types : 5
Number of agents : 16
Connection pool size : 16
Statement cache size : 40
Auto commit : false
Sleep time : 600,350,300,250,250 msec

Checkpoint segments:1024
Checkpoint timeout:5 mins

Scenario WAL generated(bytes) Compression
(bytes) TPS (tx1,tx2,tx3,tx4,tx5)
No_compress 2220787088 (~2221MB) NULL
13.3,13.3,1.3,1.3,1.3 tps
Pglz 1796213760 (~1796MB) 424573328
(19.11%) 13.1,13.1,1.3,1.3,1.3 tps
Snappy 1724171112 (~1724MB) 496615976( 22.36%)
13.2,13.2,1.3,1.3,1.3 tps
LZ4(HC) 1658941328 (~1659MB) 561845760(25.29%)
13.2,13.2,1.3,1.3,1.3 tps
FPW(off) 139384320(~139 MB) NULL
13.3,13.3,1.3,1.3,1.3 tps

As per measurement results, WAL reduction using LZ4 is close to 25% which
shows 6 percent increase in WAL reduction when compared to pglz . WAL
reduction in snappy is close to 22 % .
The numbers for compression using LZ4 and Snappy doesn’t seem to be very
high as compared to pglz for given workload. This can be due to
in-compressible nature of the TPC-C data which contains random strings

Compression does not have bad impact on the response time. In fact, response
times for Snappy, LZ4 are much better than no compression with almost ½ to
1/3 of the response times of no-compression(FPW=on) and FPW = off.
The response time order for each type of compression is
Pglz>Snappy>LZ4

Scenario Response time (tx1,tx2,tx3,tx4,tx5)
no_compress 5555,1848,4221,6791,5747 msec
pglz 4275,2659,1828,4025,3326 msec
Snappy 3790,2828,2186,1284,1120 msec
LZ4(hC) 2519,2449,1158,2066,2065 msec
FPW(off) 6234,2430,3017,5417,5885 msec

LZ4 and Snappy are almost at par with each other in terms of response time
as average response times of five types of transactions remains almost same
for both.
0001-CompressBackupBlock_snappy_lz4_pglz.patch
<http://postgresql.1045698.n5.nabble.com/file/n5805044/0001-CompressBackupBlock_snappy_lz4_pglz.patch>
0002-Support_snappy_lz4.patch
<http://postgresql.1045698.n5.nabble.com/file/n5805044/0002-Support_snappy_lz4.patch>

--
View this message in context: http://postgresql.1045698.n5.nabble.com/Compression-of-full-page-writes-tp5769039p5805044.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

On Tue, May 27, 2014 at 12:57 PM, Rahila Syed <rahilasyed(dot)90(at)gmail(dot)com> wrote:
> Hello All,
>
> 0001-CompressBackupBlock_snappy_lz4_pglz extends patch on compression of
> full page writes to include LZ4 and Snappy . Changes include making
> "compress_backup_block" GUC from boolean to enum. Value of the GUC can be
> OFF, pglz, snappy or lz4 which can be used to turn off compression or set
> the desired compression algorithm.
>
> 0002-Support_snappy_lz4 adds support for LZ4 and Snappy in PostgreSQL. It
> uses Andres’s patch for getting Makefiles working and has a few wrappers to
> make the function calls to LZ4 and Snappy compression functions and handle
> varlena datatypes.
> Patch Courtesy: Pavan Deolasee

Thanks for extending and revising the FPW-compress patch! Could you add
your patch into next CF?

> Also, compress_backup_block GUC needs to be merged with full_page_writes.

Basically I agree with you because I don't want to add new GUC very similar to
the existing one.

But could you imagine the case where full_page_writes = off. Even in this case,
FPW is forcibly written only during base backup. Such FPW also should be
compressed? Which compression algorithm should be used? If we want to
choose the algorithm for such FPW, we would not be able to merge those two
GUCs. IMO it's OK to always use the best compression algorithm for such FPW
and merge them, though.

> Tests use JDBC runner TPC-C benchmark to measure the amount of WAL
> compression ,tps and response time in each of the scenarios viz .
> Compression = OFF , pglz, LZ4 , snappy ,FPW=off

Isn't it worth measuring the recovery performance for each compression
algorithm?

Regards,

--
Fujii Masao

On 28 May 2014 15:34, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:

>> Also, compress_backup_block GUC needs to be merged with full_page_writes.
>
> Basically I agree with you because I don't want to add new GUC very similar to
> the existing one.
>
> But could you imagine the case where full_page_writes = off. Even in this case,
> FPW is forcibly written only during base backup. Such FPW also should be
> compressed? Which compression algorithm should be used? If we want to
> choose the algorithm for such FPW, we would not be able to merge those two
> GUCs. IMO it's OK to always use the best compression algorithm for such FPW
> and merge them, though.

I'd prefer a new name altogether

torn_page_protection = 'full_page_writes'
torn_page_protection = 'compressed_full_page_writes'
torn_page_protection = 'none'

this allows us to add new techniques later like

torn_page_protection = 'background_FPWs'

or

torn_page_protection = 'double_buffering'

when/if we add those new techniques

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Wed, May 28, 2014 at 04:04:13PM +0100, Simon Riggs wrote:
> On 28 May 2014 15:34, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>
> >> Also, compress_backup_block GUC needs to be merged with full_page_writes.
> >
> > Basically I agree with you because I don't want to add new GUC very similar to
> > the existing one.
> >
> > But could you imagine the case where full_page_writes = off. Even in this case,
> > FPW is forcibly written only during base backup. Such FPW also should be
> > compressed? Which compression algorithm should be used? If we want to
> > choose the algorithm for such FPW, we would not be able to merge those two
> > GUCs. IMO it's OK to always use the best compression algorithm for such FPW
> > and merge them, though.
>
> I'd prefer a new name altogether
>
> torn_page_protection = 'full_page_writes'
> torn_page_protection = 'compressed_full_page_writes'
> torn_page_protection = 'none'
>
> this allows us to add new techniques later like
>
> torn_page_protection = 'background_FPWs'
>
> or
>
> torn_page_protection = 'double_buffering'
>
> when/if we add those new techniques

Uh, how would that work if you want to compress the background_FPWs?
Use compressed_background_FPWs?

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

On 29 May 2014 01:07, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> On Wed, May 28, 2014 at 04:04:13PM +0100, Simon Riggs wrote:
>> On 28 May 2014 15:34, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>
>> >> Also, compress_backup_block GUC needs to be merged with full_page_writes.
>> >
>> > Basically I agree with you because I don't want to add new GUC very similar to
>> > the existing one.
>> >
>> > But could you imagine the case where full_page_writes = off. Even in this case,
>> > FPW is forcibly written only during base backup. Such FPW also should be
>> > compressed? Which compression algorithm should be used? If we want to
>> > choose the algorithm for such FPW, we would not be able to merge those two
>> > GUCs. IMO it's OK to always use the best compression algorithm for such FPW
>> > and merge them, though.
>>
>> I'd prefer a new name altogether
>>
>> torn_page_protection = 'full_page_writes'
>> torn_page_protection = 'compressed_full_page_writes'
>> torn_page_protection = 'none'
>>
>> this allows us to add new techniques later like
>>
>> torn_page_protection = 'background_FPWs'
>>
>> or
>>
>> torn_page_protection = 'double_buffering'
>>
>> when/if we add those new techniques
>
> Uh, how would that work if you want to compress the background_FPWs?
> Use compressed_background_FPWs?

We've currently got 1 technique for torn page protection, soon to have
2 and with a 3rd on the horizon and likely to receive effort in next
release.

It seems sensible to have just one parameter to describe the various
techniques, as suggested. I'm suggesting that we plan for how things
will look when we have the 3rd one as well.

Alternate suggestions welcome.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

>Thanks for extending and revising the FPW-compress patch! Could you add
>your patch into next CF?
Sure. I will make improvements and add it to next CF.

>Isn't it worth measuring the recovery performance for each compression
>algorithm?
Yes I will post this soon.

On Wed, May 28, 2014 at 8:04 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:

> On Tue, May 27, 2014 at 12:57 PM, Rahila Syed <rahilasyed(dot)90(at)gmail(dot)com>
> wrote:
> > Hello All,
> >
> > 0001-CompressBackupBlock_snappy_lz4_pglz extends patch on compression of
> > full page writes to include LZ4 and Snappy . Changes include making
> > "compress_backup_block" GUC from boolean to enum. Value of the GUC can be
> > OFF, pglz, snappy or lz4 which can be used to turn off compression or set
> > the desired compression algorithm.
> >
> > 0002-Support_snappy_lz4 adds support for LZ4 and Snappy in PostgreSQL. It
> > uses Andres’s patch for getting Makefiles working and has a few wrappers
> to
> > make the function calls to LZ4 and Snappy compression functions and
> handle
> > varlena datatypes.
> > Patch Courtesy: Pavan Deolasee
>
> Thanks for extending and revising the FPW-compress patch! Could you add
> your patch into next CF?
>
> > Also, compress_backup_block GUC needs to be merged with full_page_writes.
>
> Basically I agree with you because I don't want to add new GUC very
> similar to
> the existing one.
>
> But could you imagine the case where full_page_writes = off. Even in this
> case,
> FPW is forcibly written only during base backup. Such FPW also should be
> compressed? Which compression algorithm should be used? If we want to
> choose the algorithm for such FPW, we would not be able to merge those two
> GUCs. IMO it's OK to always use the best compression algorithm for such FPW
> and merge them, though.
>
> > Tests use JDBC runner TPC-C benchmark to measure the amount of WAL
> > compression ,tps and response time in each of the scenarios viz .
> > Compression = OFF , pglz, LZ4 , snappy ,FPW=off
>
> Isn't it worth measuring the recovery performance for each compression
> algorithm?
>
> Regards,
>
> --
> Fujii Masao
>

On Thu, May 29, 2014 at 11:21:44AM +0100, Simon Riggs wrote:
> > Uh, how would that work if you want to compress the background_FPWs?
> > Use compressed_background_FPWs?
>
> We've currently got 1 technique for torn page protection, soon to have
> 2 and with a 3rd on the horizon and likely to receive effort in next
> release.
>
> It seems sensible to have just one parameter to describe the various
> techniques, as suggested. I'm suggesting that we plan for how things
> will look when we have the 3rd one as well.
>
> Alternate suggestions welcome.

I was just pointing out that we might need compression to be a separate
boolean variable from the type of page tear protection. I know I am
usually anti-adding-variables, but in this case it seems trying to have
one variable control several things will lead to confusion.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

On Thu, May 29, 2014 at 7:21 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> On 29 May 2014 01:07, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>> On Wed, May 28, 2014 at 04:04:13PM +0100, Simon Riggs wrote:
>>> On 28 May 2014 15:34, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>>
>>> >> Also, compress_backup_block GUC needs to be merged with full_page_writes.
>>> >
>>> > Basically I agree with you because I don't want to add new GUC very similar to
>>> > the existing one.
>>> >
>>> > But could you imagine the case where full_page_writes = off. Even in this case,
>>> > FPW is forcibly written only during base backup. Such FPW also should be
>>> > compressed? Which compression algorithm should be used? If we want to
>>> > choose the algorithm for such FPW, we would not be able to merge those two
>>> > GUCs. IMO it's OK to always use the best compression algorithm for such FPW
>>> > and merge them, though.
>>>
>>> I'd prefer a new name altogether
>>>
>>> torn_page_protection = 'full_page_writes'
>>> torn_page_protection = 'compressed_full_page_writes'
>>> torn_page_protection = 'none'
>>>
>>> this allows us to add new techniques later like
>>>
>>> torn_page_protection = 'background_FPWs'
>>>
>>> or
>>>
>>> torn_page_protection = 'double_buffering'
>>>
>>> when/if we add those new techniques
>>
>> Uh, how would that work if you want to compress the background_FPWs?
>> Use compressed_background_FPWs?
>
> We've currently got 1 technique for torn page protection, soon to have
> 2 and with a 3rd on the horizon and likely to receive effort in next
> release.
>
> It seems sensible to have just one parameter to describe the various
> techniques, as suggested. I'm suggesting that we plan for how things
> will look when we have the 3rd one as well.
>
> Alternate suggestions welcome.

Is even compression of double buffer worthwhile? If yes, what about separating
the GUC parameter into torn_page_protection and something like
full_page_compression? ISTM that any combination of settings of those parameters
can work.

torn_page_protection = 'FPW', 'background FPW', 'none', 'double buffer'
full_page_compression = 'no', 'pglz', 'lz4', 'snappy'

Regards,

--
Fujii Masao

Hello ,

In order to facilitate changing of compression algorithms and to be able
to recover using WAL records compressed with different compression
algorithms, information about compression algorithm can be stored in WAL
record.

XLOG record header has 2 to 4 padding bytes in order to align the WAL
record. This space can be used for a new flag in order to store
information about the compression algorithm used. Like the xl_info field of
XlogRecord struct, 8 bits flag can be constructed with the lower 4 bits
of the flag used to indicate which backup block is compressed out of
0,1,2,3. Higher four bits can be used to indicate state of compression i.e
off,lz4,snappy,pglz.

The flag can be extended to incorporate more compression algorithms added
in future if any.

What is your opinion on this?

Thank you,

Rahila Syed

On Tue, May 27, 2014 at 9:27 AM, Rahila Syed <rahilasyed(dot)90(at)gmail(dot)com>
wrote:

> Hello All,
>
> 0001-CompressBackupBlock_snappy_lz4_pglz extends patch on compression of
> full page writes to include LZ4 and Snappy . Changes include making
> "compress_backup_block" GUC from boolean to enum. Value of the GUC can be
> OFF, pglz, snappy or lz4 which can be used to turn off compression or set
> the desired compression algorithm.
>
> 0002-Support_snappy_lz4 adds support for LZ4 and Snappy in PostgreSQL. It
> uses Andres’s patch for getting Makefiles working and has a few wrappers to
> make the function calls to LZ4 and Snappy compression functions and handle
> varlena datatypes.
> Patch Courtesy: Pavan Deolasee
>
> These patches serve as a way to test various compression algorithms. These
> are WIP yet. They don’t support changing compression algorithms on standby
> .
> Also, compress_backup_block GUC needs to be merged with full_page_writes.
> The patch uses LZ4 high compression(HC) variant.
> I have conducted initial tests which I would like to share and solicit
> feedback
>
> Tests use JDBC runner TPC-C benchmark to measure the amount of WAL
> compression ,tps and response time in each of the scenarios viz .
> Compression = OFF , pglz, LZ4 , snappy ,FPW=off
>
> Server specifications:
> Processors:Intel® Xeon ® Processor E5-2650 (2 GHz, 8C/16T, 20 MB) * 2 nos
> RAM: 32GB
> Disk : HDD 450GB 10K Hot Plug 2.5-inch SAS HDD * 8 nos
> 1 x 450 GB SAS HDD, 2.5-inch, 6Gb/s, 10,000 rpm
>
>
> Benchmark:
> Scale : 100
> Command :java JR /home/postgres/jdbcrunner-1.2/scripts/tpcc.js
> -sleepTime
> 600,350,300,250,250
> Warmup time : 1 sec
> Measurement time : 900 sec
> Number of tx types : 5
> Number of agents : 16
> Connection pool size : 16
> Statement cache size : 40
> Auto commit : false
> Sleep time : 600,350,300,250,250 msec
>
> Checkpoint segments:1024
> Checkpoint timeout:5 mins
>
>
> Scenario WAL generated(bytes) Compression
> (bytes) TPS (tx1,tx2,tx3,tx4,tx5)
> No_compress 2220787088 (~2221MB) NULL
> 13.3,13.3,1.3,1.3,1.3 tps
> Pglz 1796213760 (~1796MB) 424573328
> (19.11%) 13.1,13.1,1.3,1.3,1.3 tps
> Snappy 1724171112 (~1724MB) 496615976( 22.36%)
> 13.2,13.2,1.3,1.3,1.3 tps
> LZ4(HC) 1658941328 (~1659MB) 561845760(25.29%)
> 13.2,13.2,1.3,1.3,1.3 tps
> FPW(off) 139384320(~139 MB) NULL
> 13.3,13.3,1.3,1.3,1.3 tps
>
> As per measurement results, WAL reduction using LZ4 is close to 25% which
> shows 6 percent increase in WAL reduction when compared to pglz . WAL
> reduction in snappy is close to 22 % .
> The numbers for compression using LZ4 and Snappy doesn’t seem to be very
> high as compared to pglz for given workload. This can be due to
> in-compressible nature of the TPC-C data which contains random strings
>
> Compression does not have bad impact on the response time. In fact,
> response
> times for Snappy, LZ4 are much better than no compression with almost ½ to
> 1/3 of the response times of no-compression(FPW=on) and FPW = off.
> The response time order for each type of compression is
> Pglz>Snappy>LZ4
>
> Scenario Response time (tx1,tx2,tx3,tx4,tx5)
> no_compress 5555,1848,4221,6791,5747 msec
> pglz 4275,2659,1828,4025,3326 msec
> Snappy 3790,2828,2186,1284,1120 msec
> LZ4(hC) 2519,2449,1158,2066,2065 msec
> FPW(off) 6234,2430,3017,5417,5885 msec
>
> LZ4 and Snappy are almost at par with each other in terms of response time
> as average response times of five types of transactions remains almost same
> for both.
> 0001-CompressBackupBlock_snappy_lz4_pglz.patch
> <
> http://postgresql.1045698.n5.nabble.com/file/n5805044/0001-CompressBackupBlock_snappy_lz4_pglz.patch
> >
> 0002-Support_snappy_lz4.patch
> <
> http://postgresql.1045698.n5.nabble.com/file/n5805044/0002-Support_snappy_lz4.patch
> >
>
>
>
>
> --
> View this message in context:
> http://postgresql.1045698.n5.nabble.com/Compression-of-full-page-writes-tp5769039p5805044.html
> Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

On Tue, Jun 10, 2014 at 11:49 PM, Rahila Syed <rahilasyed90(at)gmail(dot)com> wrote:
> Hello ,
>
>
> In order to facilitate changing of compression algorithms and to be able to
> recover using WAL records compressed with different compression algorithms,
> information about compression algorithm can be stored in WAL record.
>
> XLOG record header has 2 to 4 padding bytes in order to align the WAL
> record. This space can be used for a new flag in order to store information
> about the compression algorithm used. Like the xl_info field of XlogRecord
> struct, 8 bits flag can be constructed with the lower 4 bits of the flag
> used to indicate which backup block is compressed out of 0,1,2,3. Higher
> four bits can be used to indicate state of compression i.e
> off,lz4,snappy,pglz.
>
> The flag can be extended to incorporate more compression algorithms added in
> future if any.
>
> What is your opinion on this?
-1 for any additional bytes in WAL record to control such things,
having one single compression that we know performs well and relying
on it makes the life of user and developer easier.
--
Michael

On Wed, Jun 11, 2014 at 10:05 AM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Tue, Jun 10, 2014 at 11:49 PM, Rahila Syed <rahilasyed90(at)gmail(dot)com> wrote:
>> Hello ,
>>
>>
>> In order to facilitate changing of compression algorithms and to be able to
>> recover using WAL records compressed with different compression algorithms,
>> information about compression algorithm can be stored in WAL record.
>>
>> XLOG record header has 2 to 4 padding bytes in order to align the WAL
>> record. This space can be used for a new flag in order to store information
>> about the compression algorithm used. Like the xl_info field of XlogRecord
>> struct, 8 bits flag can be constructed with the lower 4 bits of the flag
>> used to indicate which backup block is compressed out of 0,1,2,3. Higher
>> four bits can be used to indicate state of compression i.e
>> off,lz4,snappy,pglz.
>>
>> The flag can be extended to incorporate more compression algorithms added in
>> future if any.
>>
>> What is your opinion on this?
> -1 for any additional bytes in WAL record to control such things,
> having one single compression that we know performs well and relying
> on it makes the life of user and developer easier.

IIUC even when we adopt only one algorithm, additional at least one bit is
necessary to see whether this backup block is compressed or not.

This flag is necessary only for backup block, so there is no need to use
the header of each WAL record. What about just using the backup block
header?

Regards,

--
Fujii Masao

On Wed, Jun 11, 2014 at 4:19 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:

>
> IIUC even when we adopt only one algorithm, additional at least one bit is
> necessary to see whether this backup block is compressed or not.
>
> This flag is necessary only for backup block, so there is no need to use
> the header of each WAL record. What about just using the backup block
> header?
>
>
+1. We can also steal a few bits from ForkNumber field in the backup block
header if required.

Thanks,
Pavan

--
Pavan Deolasee
http://www.linkedin.com/in/pavandeolasee

Hello,

The attached patch named CompressBackupBlock_snappy_lz4_pglz accomplishes
compression of FPW in WAL using pglz ,LZ4 and Snappy. This serves as a
means to test performance of various compression algorithms for FPW
compression
Minor correction in check for compression/decompression is made since the
last time it was posted.

Patch named Support-for-lz4-and-snappy adds support for LZ4 and Snappy in
PostgreSQL.

Below are the performance numbers taken for various values of
compress_backup_block GUC parameter.

Scenario Amount of WAL(bytes) Compression
(bytes) WALRecovery time(secs) TPS

FPW(on)Compression(Off) 1393681216 (~1394MB) NA
17 s 15.8,15.8,1.6,1.6,1.6
tps

Pglz 1192524560 (~1193 MB)
14% 17 s
15.6,15.6,1.6,1.6,1.6
tps

LZ4 1124745880 (~1125MB)
19.2% 16 s
15.7,15.7,1.6,1.6,1.6
tps

Snappy 1123117704 (~1123MB) 19.4%
17 s
15.6,15.6,1.6,1.6,1.6
tps

FPW (off) 171287384 ( ~171MB)
NA 12 s
16.0,16.0,1.6,1.6,1.6
tps

Compression ratios of LZ4 and Snappy are almost at par for given workload.
The nature of TPC-C type of data used is highly incompressible which
explains the low compression ratios.

Turning compression on reduces tps overall. TPS numbers for LZ4 is slightly
better than pglz and snappy.

Recovery(decompression) speed of LZ4 is slightly faster than Snappy.

Overall LZ4 scores over Snappy and pglz in terms of recovery
(decompression) speed ,TPS and response times. Also, compression of LZ4 is
at par with Snappy.
Server specifications:
Processors:Intel® Xeon ® Processor E5-2650 (2 GHz, 8C/16T, 20 MB) * 2 nos
RAM: 32GB
Disk : HDD 450GB 10K Hot Plug 2.5-inch SAS HDD * 8 nos
1 x 450 GB SAS HDD, 2.5-inch, 6Gb/s, 10,000 rpm

Benchmark:
Scale : 16
Command :java JR /home/postgres/jdbcrunner-1.2/scripts/tpcc.js
-sleepTime 550,250,250,200,200
Warmup time : 1 sec
Measurement time : 900 sec
Number of tx types : 5
Number of agents : 16
Connection pool size : 16
Statement cache size : 40
Auto commit : false

Checkpoint segments:1024
Checkpoint timeout:5 mins

Limitations of the current patch:
1. The patch currently compresses entire backup block inclusive of ‘hole’
unlike normal code which backs up the part before and after the
hole separately. There can be performance issues when ‘hole’ is not filled
with zeros. Hence separately compressing parts of block before and
after hole can be considered.
2. Patch currently relies on ‘compress_backup_block’ GUC parameter to check
if FPW is compressed or not. Information about whether FPW is compressed
and which compression algorithm is used can be included in WAL record
header . This will enable switching compression off and changing
compression algorithm whenever desired.
3. Extending decompression logic to pg_xlogdump.

On Tue, May 27, 2014 at 9:27 AM, Rahila Syed <rahilasyed(dot)90(at)gmail(dot)com>
wrote:

> Hello All,
>
> 0001-CompressBackupBlock_snappy_lz4_pglz extends patch on compression of
> full page writes to include LZ4 and Snappy . Changes include making
> "compress_backup_block" GUC from boolean to enum. Value of the GUC can be
> OFF, pglz, snappy or lz4 which can be used to turn off compression or set
> the desired compression algorithm.
>
> 0002-Support_snappy_lz4 adds support for LZ4 and Snappy in PostgreSQL. It
> uses Andres’s patch for getting Makefiles working and has a few wrappers to
> make the function calls to LZ4 and Snappy compression functions and handle
> varlena datatypes.
> Patch Courtesy: Pavan Deolasee
>
> These patches serve as a way to test various compression algorithms. These
> are WIP yet. They don’t support changing compression algorithms on standby
> .
> Also, compress_backup_block GUC needs to be merged with full_page_writes.
> The patch uses LZ4 high compression(HC) variant.
> I have conducted initial tests which I would like to share and solicit
> feedback
>
> Tests use JDBC runner TPC-C benchmark to measure the amount of WAL
> compression ,tps and response time in each of the scenarios viz .
> Compression = OFF , pglz, LZ4 , snappy ,FPW=off
>
> Server specifications:
> Processors:Intel® Xeon ® Processor E5-2650 (2 GHz, 8C/16T, 20 MB) * 2 nos
> RAM: 32GB
> Disk : HDD 450GB 10K Hot Plug 2.5-inch SAS HDD * 8 nos
> 1 x 450 GB SAS HDD, 2.5-inch, 6Gb/s, 10,000 rpm
>
>
> Benchmark:
> Scale : 100
> Command :java JR /home/postgres/jdbcrunner-1.2/scripts/tpcc.js
> -sleepTime
> 600,350,300,250,250
> Warmup time : 1 sec
> Measurement time : 900 sec
> Number of tx types : 5
> Number of agents : 16
> Connection pool size : 16
> Statement cache size : 40
> Auto commit : false
> Sleep time : 600,350,300,250,250 msec
>
> Checkpoint segments:1024
> Checkpoint timeout:5 mins
>
>
> Scenario WAL generated(bytes) Compression
> (bytes) TPS (tx1,tx2,tx3,tx4,tx5)
> No_compress 2220787088 (~2221MB) NULL
> 13.3,13.3,1.3,1.3,1.3 tps
> Pglz 1796213760 (~1796MB) 424573328
> (19.11%) 13.1,13.1,1.3,1.3,1.3 tps
> Snappy 1724171112 (~1724MB) 496615976( 22.36%)
> 13.2,13.2,1.3,1.3,1.3 tps
> LZ4(HC) 1658941328 (~1659MB) 561845760(25.29%)
> 13.2,13.2,1.3,1.3,1.3 tps
> FPW(off) 139384320(~139 MB) NULL
> 13.3,13.3,1.3,1.3,1.3 tps
>
> As per measurement results, WAL reduction using LZ4 is close to 25% which
> shows 6 percent increase in WAL reduction when compared to pglz . WAL
> reduction in snappy is close to 22 % .
> The numbers for compression using LZ4 and Snappy doesn’t seem to be very
> high as compared to pglz for given workload. This can be due to
> in-compressible nature of the TPC-C data which contains random strings
>
> Compression does not have bad impact on the response time. In fact,
> response
> times for Snappy, LZ4 are much better than no compression with almost ½ to
> 1/3 of the response times of no-compression(FPW=on) and FPW = off.
> The response time order for each type of compression is
> Pglz>Snappy>LZ4
>
> Scenario Response time (tx1,tx2,tx3,tx4,tx5)
> no_compress 5555,1848,4221,6791,5747 msec
> pglz 4275,2659,1828,4025,3326 msec
> Snappy 3790,2828,2186,1284,1120 msec
> LZ4(hC) 2519,2449,1158,2066,2065 msec
> FPW(off) 6234,2430,3017,5417,5885 msec
>
> LZ4 and Snappy are almost at par with each other in terms of response time
> as average response times of five types of transactions remains almost same
> for both.
> 0001-CompressBackupBlock_snappy_lz4_pglz.patch
> <
> http://postgresql.1045698.n5.nabble.com/file/n5805044/0001-CompressBackupBlock_snappy_lz4_pglz.patch
> >
> 0002-Support_snappy_lz4.patch
> <
> http://postgresql.1045698.n5.nabble.com/file/n5805044/0002-Support_snappy_lz4.patch
> >
>
>
>
>
> --
> View this message in context:
> http://postgresql.1045698.n5.nabble.com/Compression-of-full-page-writes-tp5769039p5805044.html
> Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

At 2014-06-13 20:07:29 +0530, rahilasyed90(at)gmail(dot)com wrote:
>
> Patch named Support-for-lz4-and-snappy adds support for LZ4 and Snappy
> in PostgreSQL.

I haven't looked at this in any detail yet, but I note that the patch
creates src/common/lz4/.travis.yml, which it shouldn't.

I have a few preliminary comments about your patch.

> @@ -84,6 +87,7 @@ bool XLogArchiveMode = false;
> char *XLogArchiveCommand = NULL;
> bool EnableHotStandby = false;
> bool fullPageWrites = true;
> +int compress_backup_block = false;

I think compress_backup_block should be initialised to
BACKUP_BLOCK_COMPRESSION_OFF. (But see below.)

> + for (j = 0; j < XLR_MAX_BKP_BLOCKS; j++)
> + compressed_pages[j] = (char *) malloc(buffer_size);

Shouldn't this use palloc?

> + * Create a compressed version of a backup block
> + *
> + * If successful, return a compressed result and set 'len' to its length.
> + * Otherwise (ie, compressed result is actually bigger than original),
> + * return NULL.
> + */
> +static char *
> +CompressBackupBlock(char *page, uint32 orig_len, char *dest, uint32 *len)
> +{

First, the calling convention is a bit strange. I understand that you're
pre-allocating compressed_pages[] so as to avoid repeated allocations;
and that you're doing it outside CompressBackupBlock so as to avoid
passing in the index i. But the result is a little weird.

At the very minimum, I would move the "if (!compressed_pages_allocated)"
block outside the "for (i = 0; i < XLR_MAX_BKP_BLOCKS; i++)" loop, and
add some comments. I think we could live with that.

But I'm not at all fond of the code in this function either. I'd write
it like this:

struct varlena *buf = (struct varlena *) dest;

if (compress_backup_block = BACKUP_BLOCK_COMPRESSION_SNAPPY)
{
if (pg_snappy_compress(page, BLCKSZ, buf) == EIO)
return NULL;
}
else if (compress_backup_block = BACKUP_BLOCK_COMPRESSION_LZ4)
{
if (pg_LZ4_compress(page, BLCKSZ, buf) == 0)
return NULL;
}
else if (compress_backup_block = BACKUP_BLOCK_COMPRESSION_PGLZ)
{
if (pglz_compress(page, BLCKSZ, (PGLZ_Header *) buf,
PGLZ_strategy_default) != 0)
return NULL;
}
else
elog(ERROR, "Wrong value for compress_backup_block GUC");

/*
* …comment about insisting on saving at least two bytes…
*/

if (VARSIZE(buf) >= orig_len - 2)
return NULL;

*len = VARHDRSIZE + VARSIZE(buf);

return buf;

I guess it doesn't matter *too* much if the intention is to have all
these compression algorithms only during development/testing and pick
just one in the end. But the above is considerably easier to read in
the meanwhile.

If we were going to keep multiple compression algorithms around, I'd be
inclined to create a "pg_compress(…, compression_algorithm)" function to
hide these return-value differences from the callers.

> + else if (VARATT_IS_COMPRESSED((struct varlena *) blk) && compress_backup_block!=BACKUP_BLOCK_COMPRESSION_OFF)
> + {
> + if (compress_backup_block == BACKUP_BLOCK_COMPRESSION_SNAPPY)
> + {
> + int ret;
> + size_t compressed_length = VARSIZE((struct varlena *) blk) - VARHDRSZ;
> + char *compressed_data = (char *)VARDATA((struct varlena *) blk);
> + size_t s_uncompressed_length;
> +
> + ret = snappy_uncompressed_length(compressed_data,
> + compressed_length,
> + &s_uncompressed_length);
> + if (!ret)
> + elog(ERROR, "snappy: failed to determine compression length");
> + if (BLCKSZ != s_uncompressed_length)
> + elog(ERROR, "snappy: compression size mismatch %d != %zu",
> + BLCKSZ, s_uncompressed_length);
> +
> + ret = snappy_uncompress(compressed_data,
> + compressed_length,
> + page);
> + if (ret != 0)
> + elog(ERROR, "snappy: decompression failed: %d", ret);
> + }

…and a "pg_decompress()" function that does error checking.

> +static const struct config_enum_entry backup_block_compression_options[] = {
> + {"off", BACKUP_BLOCK_COMPRESSION_OFF, false},
> + {"false", BACKUP_BLOCK_COMPRESSION_OFF, true},
> + {"no", BACKUP_BLOCK_COMPRESSION_OFF, true},
> + {"0", BACKUP_BLOCK_COMPRESSION_OFF, true},
> + {"pglz", BACKUP_BLOCK_COMPRESSION_PGLZ, true},
> + {"snappy", BACKUP_BLOCK_COMPRESSION_SNAPPY, true},
> + {"lz4", BACKUP_BLOCK_COMPRESSION_LZ4, true},
> + {NULL, 0, false}
> +};

Finally, I don't like the name "compress_backup_block".

1. It should have been plural (compress_backup_blockS).

2. Looking at the enum values, "backup_block_compression = x" would be a
better name anyway…

3. But we don't use the term "backup block" anywhere in the
documentation, and it's very likely to confuse people.

I don't mind the suggestion elsewhere in this thread to use
"full_page_compression = y" (as a setting alongside
"torn_page_protection = x").

I haven't tried the patch (other than applying and building it) yet. I
will do so after I hear what you and others think of the above points.

-- Abhijit

On Tue, Jun 17, 2014 at 8:47 AM, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com> wrote:
> if (compress_backup_block = BACKUP_BLOCK_COMPRESSION_SNAPPY)

You mean == right?

At 2014-06-17 15:31:33 -0300, klaussfreire(at)gmail(dot)com wrote:
>
> On Tue, Jun 17, 2014 at 8:47 AM, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com> wrote:
> > if (compress_backup_block = BACKUP_BLOCK_COMPRESSION_SNAPPY)
>
> You mean == right?

Of course. Thanks.

-- Abhijit

Hello ,

>I have a few preliminary comments about your patch
Thank you for review comments.

>the patch creates src/common/lz4/.travis.yml, which it shouldn't.
Agree. I will remove it.

>Shouldn't this use palloc?
palloc() is disallowed in critical sections and we are already in CS while
executing this code. So we use malloc(). It's OK since the memory is
allocated just once per session and it stays till the end.

>At the very minimum, I would move the "if (!compressed_pages_allocated)"
>block outside the "for (i = 0; i < XLR_MAX_BKP_BLOCKS; i++)" loop,.

Yes , the code for allocating memory is being executed just once for each
run of the program so it can be taken out of the for loop. But as the
condition
if (compress_backup_block != BACKUP_BLOCK_COMPRESSION_OFF &&
!compressed_pages_allocated) evaluates to be true
for just the first loop , I am not sure if the change will be a significant
improvement from performance point of view except it will save few
condition checks.

>and
>add some comments. I think we could live with that
I will add comments.

>If we were going to keep multiple compression algorithms around, I'd be
>inclined to create a "pg_compress(…, compression_algorithm)" function to
>hide these return-value differences from the callers. and a
"pg_decompress()" function that does error checking

+1 for abstracting out the differences in the return values and arguments
and provide a common interface for all compression algorithms.

> if (compress_backup_block = BACKUP_BLOCK_COMPRESSION_SNAPPY)
> {
if (pg_snappy_compress(page, BLCKSZ, buf) == EIO)
return NULL;
> }
> else if (compress_backup_block = BACKUP_BLOCK_COMPRESSION_LZ4)
> {
> if (pg_LZ4_compress(page, BLCKSZ, buf) == 0)
> return NULL;
> }
> else if (compress_backup_block = BACKUP_BLOCK_COMPRESSION_PGLZ)
> {
> if (pglz_compress(page, BLCKSZ, (PGLZ_Header *) buf,
> PGLZ_strategy_default) != 0)
> return NULL;
> }
> else
> elog(ERROR, "Wrong value for compress_backup_block GUC");

> /*
> * …comment about insisting on saving at least two bytes…
> */

> if (VARSIZE(buf) >= orig_len - 2)
> return NULL;

> *len = VARHDRSIZE + VARSIZE(buf);

> return buf;
>I guess it doesn't matter *too* much if the intention is to have all
>these compression algorithms only during development/testing and pick
>just one in the end. But the above is considerably easier to read in
>the meanwhile.

The above version is better as it avoids goto statement.

>I don't mind the suggestion elsewhere in this thread to use
>"full_page_compression = y" (as a setting alongside
>"torn_page_protection = x").

This change of GUC is in the ToDo for this patch.

Thank you,

Rahila

On Tue, Jun 17, 2014 at 5:17 PM, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com>
wrote:

> At 2014-06-13 20:07:29 +0530, rahilasyed90(at)gmail(dot)com wrote:
> >
> > Patch named Support-for-lz4-and-snappy adds support for LZ4 and Snappy
> > in PostgreSQL.
>
> I haven't looked at this in any detail yet, but I note that the patch
> creates src/common/lz4/.travis.yml, which it shouldn't.
>
> I have a few preliminary comments about your patch.
>
> > @@ -84,6 +87,7 @@ bool XLogArchiveMode = false;
> > char *XLogArchiveCommand = NULL;
> > bool EnableHotStandby = false;
> > bool fullPageWrites = true;
> > +int compress_backup_block = false;
>
> I think compress_backup_block should be initialised to
> BACKUP_BLOCK_COMPRESSION_OFF. (But see below.)
>
> > + for (j = 0; j < XLR_MAX_BKP_BLOCKS; j++)
> > + compressed_pages[j] = (char *)
> malloc(buffer_size);
>
> Shouldn't this use palloc?
>
> > + * Create a compressed version of a backup block
> > + *
> > + * If successful, return a compressed result and set 'len' to its
> length.
> > + * Otherwise (ie, compressed result is actually bigger than original),
> > + * return NULL.
> > + */
> > +static char *
> > +CompressBackupBlock(char *page, uint32 orig_len, char *dest, uint32
> *len)
> > +{
>
> First, the calling convention is a bit strange. I understand that you're
> pre-allocating compressed_pages[] so as to avoid repeated allocations;
> and that you're doing it outside CompressBackupBlock so as to avoid
> passing in the index i. But the result is a little weird.
>
> At the very minimum, I would move the "if (!compressed_pages_allocated)"
> block outside the "for (i = 0; i < XLR_MAX_BKP_BLOCKS; i++)" loop, and
> add some comments. I think we could live with that.
>
> But I'm not at all fond of the code in this function either. I'd write
> it like this:
>
> struct varlena *buf = (struct varlena *) dest;
>
> if (compress_backup_block = BACKUP_BLOCK_COMPRESSION_SNAPPY)
> {
> if (pg_snappy_compress(page, BLCKSZ, buf) == EIO)
> return NULL;
> }
> else if (compress_backup_block = BACKUP_BLOCK_COMPRESSION_LZ4)
> {
> if (pg_LZ4_compress(page, BLCKSZ, buf) == 0)
> return NULL;
> }
> else if (compress_backup_block = BACKUP_BLOCK_COMPRESSION_PGLZ)
> {
> if (pglz_compress(page, BLCKSZ, (PGLZ_Header *) buf,
> PGLZ_strategy_default) != 0)
> return NULL;
> }
> else
> elog(ERROR, "Wrong value for compress_backup_block GUC");
>
> /*
> * …comment about insisting on saving at least two bytes…
> */
>
> if (VARSIZE(buf) >= orig_len - 2)
> return NULL;
>
> *len = VARHDRSIZE + VARSIZE(buf);
>
> return buf;
>
> I guess it doesn't matter *too* much if the intention is to have all
> these compression algorithms only during development/testing and pick
> just one in the end. But the above is considerably easier to read in
> the meanwhile.
>
> If we were going to keep multiple compression algorithms around, I'd be
> inclined to create a "pg_compress(…, compression_algorithm)" function to
> hide these return-value differences from the callers.
>
> > + else if (VARATT_IS_COMPRESSED((struct varlena *) blk) &&
> compress_backup_block!=BACKUP_BLOCK_COMPRESSION_OFF)
> > + {
> > + if (compress_backup_block ==
> BACKUP_BLOCK_COMPRESSION_SNAPPY)
> > + {
> > + int ret;
> > + size_t compressed_length = VARSIZE((struct varlena
> *) blk) - VARHDRSZ;
> > + char *compressed_data = (char *)VARDATA((struct
> varlena *) blk);
> > + size_t s_uncompressed_length;
> > +
> > + ret = snappy_uncompressed_length(compressed_data,
> > + compressed_length,
> > + &s_uncompressed_length);
> > + if (!ret)
> > + elog(ERROR, "snappy: failed to determine
> compression length");
> > + if (BLCKSZ != s_uncompressed_length)
> > + elog(ERROR, "snappy: compression size
> mismatch %d != %zu",
> > + BLCKSZ,
> s_uncompressed_length);
> > +
> > + ret = snappy_uncompress(compressed_data,
> > + compressed_length,
> > + page);
> > + if (ret != 0)
> > + elog(ERROR, "snappy: decompression failed:
> %d", ret);
> > + }
>
> …and a "pg_decompress()" function that does error checking.
>
> > +static const struct config_enum_entry
> backup_block_compression_options[] = {
> > + {"off", BACKUP_BLOCK_COMPRESSION_OFF, false},
> > + {"false", BACKUP_BLOCK_COMPRESSION_OFF, true},
> > + {"no", BACKUP_BLOCK_COMPRESSION_OFF, true},
> > + {"0", BACKUP_BLOCK_COMPRESSION_OFF, true},
> > + {"pglz", BACKUP_BLOCK_COMPRESSION_PGLZ, true},
> > + {"snappy", BACKUP_BLOCK_COMPRESSION_SNAPPY, true},
> > + {"lz4", BACKUP_BLOCK_COMPRESSION_LZ4, true},
> > + {NULL, 0, false}
> > +};
>
> Finally, I don't like the name "compress_backup_block".
>
> 1. It should have been plural (compress_backup_blockS).
>
> 2. Looking at the enum values, "backup_block_compression = x" would be a
> better name anyway…
>
> 3. But we don't use the term "backup block" anywhere in the
> documentation, and it's very likely to confuse people.
>
> I don't mind the suggestion elsewhere in this thread to use
> "full_page_compression = y" (as a setting alongside
> "torn_page_protection = x").
>
> I haven't tried the patch (other than applying and building it) yet. I
> will do so after I hear what you and others think of the above points.
>
> -- Abhijit
>

On 2014-06-18 18:10:34 +0530, Rahila Syed wrote:
> Hello ,
>
> >I have a few preliminary comments about your patch
> Thank you for review comments.
>
> >the patch creates src/common/lz4/.travis.yml, which it shouldn't.
> Agree. I will remove it.
>
> >Shouldn't this use palloc?
> palloc() is disallowed in critical sections and we are already in CS while
> executing this code. So we use malloc(). It's OK since the memory is
> allocated just once per session and it stays till the end.

malloc() isn't allowed either. You'll need to make sure all memory is
allocated beforehand

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

At 2014-06-18 18:10:34 +0530, rahilasyed90(at)gmail(dot)com wrote:
>
> palloc() is disallowed in critical sections and we are already in CS
> while executing this code. So we use malloc().

Are these allocations actually inside a critical section? It seems to me
that the critical section starts further down, but perhaps I am missing
something.

Second, as Andres says, you shouldn't malloc() inside a critical section
either; and anyway, certainly not without checking the return value.

> I am not sure if the change will be a significant improvement from
> performance point of view except it will save few condition checks.

Moving that allocation out of the outer for loop it's currently in is
*nothing* to do with performance, but about making the code easier to
read.

-- Abhijit

At 2014-06-18 18:25:34 +0530, ams(at)2ndQuadrant(dot)com wrote:
>
> Are these allocations actually inside a critical section? It seems to me
> that the critical section starts further down, but perhaps I am missing
> something.

OK, I was missing that XLogInsert() itself can be called from inside a
critical section. So the allocation has to be moved somewhere else
altogether.

-- Abhijit

On Wed, Jun 18, 2014 at 6:25 PM, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com>
wrote:

> At 2014-06-18 18:10:34 +0530, rahilasyed90(at)gmail(dot)com wrote:
> >
> > palloc() is disallowed in critical sections and we are already in CS
> > while executing this code. So we use malloc().
>
> Are these allocations actually inside a critical section? It seems to me
> that the critical section starts further down, but perhaps I am missing
> something.
>
>
ISTM XLogInsert() itself is called from other critical sections. See
heapam.c for example.

> Second, as Andres says, you shouldn't malloc() inside a critical section
> either; and anyway, certainly not without checking the return value.
>
>
I was actually surprised to see Andreas comment. But he is right. OOM
inside CS will result in a PANIC. I wonder if we can or if we really do
enforce that though. The code within #ifdef WAL_DEBUG in the same function
is surely doing a palloc(). That will be caught since there is an assert
inside palloc(). May be nobody tried building with WAL_DEBUG since that
assert was added.

May be Rahila can move that code to InitXLogAccess or even better check for
malloc() return value and proceed without compression. There is code in
snappy.c which will need similar handling, if we decide to finally add that
to core.

> I am not sure if the change will be a significant improvement from
> > performance point of view except it will save few condition checks.
>
> Moving that allocation out of the outer for loop it's currently in is
> *nothing* to do with performance, but about making the code easier to
> read.
>
>
+1.

Thanks,
Pavan

--
Pavan Deolasee
http://www.linkedin.com/in/pavandeolasee

Hello,

Updated version of patches are attached.
Changes are as follows
1. Improved readability of the code as per the review comments.
2. Addition of block_compression field in BkpBlock structure to store
information about compression of block. This provides for switching
compression on/off and changing compression algorithm as required.
3.Handling of OOM in critical section by checking for return value of
malloc and proceeding without compression of FPW if return value is NULL.

Thank you,
Rahila Syed

On Tue, Jun 17, 2014 at 5:17 PM, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com>
wrote:

> At 2014-06-13 20:07:29 +0530, rahilasyed90(at)gmail(dot)com wrote:
> >
> > Patch named Support-for-lz4-and-snappy adds support for LZ4 and Snappy
> > in PostgreSQL.
>
> I haven't looked at this in any detail yet, but I note that the patch
> creates src/common/lz4/.travis.yml, which it shouldn't.
>
> I have a few preliminary comments about your patch.
>
> > @@ -84,6 +87,7 @@ bool XLogArchiveMode = false;
> > char *XLogArchiveCommand = NULL;
> > bool EnableHotStandby = false;
> > bool fullPageWrites = true;
> > +int compress_backup_block = false;
>
> I think compress_backup_block should be initialised to
> BACKUP_BLOCK_COMPRESSION_OFF. (But see below.)
>
> > + for (j = 0; j < XLR_MAX_BKP_BLOCKS; j++)
> > + compressed_pages[j] = (char *)
> malloc(buffer_size);
>
> Shouldn't this use palloc?
>
> > + * Create a compressed version of a backup block
> > + *
> > + * If successful, return a compressed result and set 'len' to its
> length.
> > + * Otherwise (ie, compressed result is actually bigger than original),
> > + * return NULL.
> > + */
> > +static char *
> > +CompressBackupBlock(char *page, uint32 orig_len, char *dest, uint32
> *len)
> > +{
>
> First, the calling convention is a bit strange. I understand that you're
> pre-allocating compressed_pages[] so as to avoid repeated allocations;
> and that you're doing it outside CompressBackupBlock so as to avoid
> passing in the index i. But the result is a little weird.
>
> At the very minimum, I would move the "if (!compressed_pages_allocated)"
> block outside the "for (i = 0; i < XLR_MAX_BKP_BLOCKS; i++)" loop, and
> add some comments. I think we could live with that.
>
> But I'm not at all fond of the code in this function either. I'd write
> it like this:
>
> struct varlena *buf = (struct varlena *) dest;
>
> if (compress_backup_block = BACKUP_BLOCK_COMPRESSION_SNAPPY)
> {
> if (pg_snappy_compress(page, BLCKSZ, buf) == EIO)
> return NULL;
> }
> else if (compress_backup_block = BACKUP_BLOCK_COMPRESSION_LZ4)
> {
> if (pg_LZ4_compress(page, BLCKSZ, buf) == 0)
> return NULL;
> }
> else if (compress_backup_block = BACKUP_BLOCK_COMPRESSION_PGLZ)
> {
> if (pglz_compress(page, BLCKSZ, (PGLZ_Header *) buf,
> PGLZ_strategy_default) != 0)
> return NULL;
> }
> else
> elog(ERROR, "Wrong value for compress_backup_block GUC");
>
> /*
> * …comment about insisting on saving at least two bytes…
> */
>
> if (VARSIZE(buf) >= orig_len - 2)
> return NULL;
>
> *len = VARHDRSIZE + VARSIZE(buf);
>
> return buf;
>
> I guess it doesn't matter *too* much if the intention is to have all
> these compression algorithms only during development/testing and pick
> just one in the end. But the above is considerably easier to read in
> the meanwhile.
>
> If we were going to keep multiple compression algorithms around, I'd be
> inclined to create a "pg_compress(…, compression_algorithm)" function to
> hide these return-value differences from the callers.
>
> > + else if (VARATT_IS_COMPRESSED((struct varlena *) blk) &&
> compress_backup_block!=BACKUP_BLOCK_COMPRESSION_OFF)
> > + {
> > + if (compress_backup_block ==
> BACKUP_BLOCK_COMPRESSION_SNAPPY)
> > + {
> > + int ret;
> > + size_t compressed_length = VARSIZE((struct varlena
> *) blk) - VARHDRSZ;
> > + char *compressed_data = (char *)VARDATA((struct
> varlena *) blk);
> > + size_t s_uncompressed_length;
> > +
> > + ret = snappy_uncompressed_length(compressed_data,
> > + compressed_length,
> > + &s_uncompressed_length);
> > + if (!ret)
> > + elog(ERROR, "snappy: failed to determine
> compression length");
> > + if (BLCKSZ != s_uncompressed_length)
> > + elog(ERROR, "snappy: compression size
> mismatch %d != %zu",
> > + BLCKSZ,
> s_uncompressed_length);
> > +
> > + ret = snappy_uncompress(compressed_data,
> > + compressed_length,
> > + page);
> > + if (ret != 0)
> > + elog(ERROR, "snappy: decompression failed:
> %d", ret);
> > + }
>
> …and a "pg_decompress()" function that does error checking.
>
> > +static const struct config_enum_entry
> backup_block_compression_options[] = {
> > + {"off", BACKUP_BLOCK_COMPRESSION_OFF, false},
> > + {"false", BACKUP_BLOCK_COMPRESSION_OFF, true},
> > + {"no", BACKUP_BLOCK_COMPRESSION_OFF, true},
> > + {"0", BACKUP_BLOCK_COMPRESSION_OFF, true},
> > + {"pglz", BACKUP_BLOCK_COMPRESSION_PGLZ, true},
> > + {"snappy", BACKUP_BLOCK_COMPRESSION_SNAPPY, true},
> > + {"lz4", BACKUP_BLOCK_COMPRESSION_LZ4, true},
> > + {NULL, 0, false}
> > +};
>
> Finally, I don't like the name "compress_backup_block".
>
> 1. It should have been plural (compress_backup_blockS).
>
> 2. Looking at the enum values, "backup_block_compression = x" would be a
> better name anyway…
>
> 3. But we don't use the term "backup block" anywhere in the
> documentation, and it's very likely to confuse people.
>
> I don't mind the suggestion elsewhere in this thread to use
> "full_page_compression = y" (as a setting alongside
> "torn_page_protection = x").
>
> I haven't tried the patch (other than applying and building it) yet. I
> will do so after I hear what you and others think of the above points.
>
> -- Abhijit
>

On Fri, Jul 4, 2014 at 4:58 AM, Rahila Syed <rahilasyed90(at)gmail(dot)com> wrote:
> Hello,
>
> Updated version of patches are attached.
> Changes are as follows
> 1. Improved readability of the code as per the review comments.
> 2. Addition of block_compression field in BkpBlock structure to store
> information about compression of block. This provides for switching
> compression on/off and changing compression algorithm as required.
> 3.Handling of OOM in critical section by checking for return value of malloc
> and proceeding without compression of FPW if return value is NULL.

Thanks for updating the patches!

But 0002-CompressBackupBlock_snappy_lz4_pglz-2.patch doesn't seem to
be able to apply to HEAD cleanly.

-----------------------------------------------
$ git am ~/Desktop/0001-Support-for-LZ4-and-Snappy-2.patch
Applying: Support for LZ4 and Snappy-2