Re: [REVIEW] Re: Compression of full-page-writes
| От | Rahila Syed |
|---|---|
| Тема | Re: [REVIEW] Re: Compression of full-page-writes |
| Дата | |
| Msg-id | CAH2L28sVyLe=GkHF6CBVKV1jZz2wL2b6J5eevScbG_erEQpARw@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: [REVIEW] Re: Compression of full-page-writes (Abhijit Menon-Sen <ams@2ndQuadrant.com>) |
| Ответы |
Re: [REVIEW] Re: Compression of full-page-writes
|
| Список | pgsql-hackers |
Thank you for review comments.
>There are still numerous formatting changes required, e.g. spaces around
>"=" and correct formatting of comments. And "git diff --check" still has
>a few whitespace problems. I won't point these out one by one, but maybe
>you should run pgindent
>"=" and correct formatting of comments. And "git diff --check" still has
>a few whitespace problems. I won't point these out one by one, but maybe
>you should run pgindent
I will do this.
>Could you look into his suggestions of other places to do the
>allocation, please?
I will get back to you on this
>Wouldn't it be better to set
> bkpb->block_compression = compress_backup_block;
>once earlier instead of setting it that way once and setting it to
>BACKUP_BLOCK_COMPRESSION_OFF in two other places
Yes.
If you're using VARATT_IS_COMPRESSED() to detect compression, don't you
need SET_VARSIZE_COMPRESSED() in CompressBackupBlock? pglz_compress()
does it for you, but the other two algorithms don't.
does it for you, but the other two algorithms don't.
Yes we need SET_VARSIZE_COMPRESSED. It is present in wrappers around snappy and LZ4 namely pg_snappy_compress and pg_LZ4_compress.
>But now that you've added bkpb.block_compression, you should be able to
>avoid VARATT_IS_COMPRESSED() altogether, unless I'm missing something.
>What do you think?
>avoid VARATT_IS_COMPRESSED() altogether, unless I'm missing something.
>What do you think?
You are right. It can be removed.
Thank you,
On Fri, Jul 4, 2014 at 9:35 PM, Abhijit Menon-Sen <ams@2ndquadrant.com> wrote:
At 2014-07-04 21:02:33 +0530, ams@2ndQuadrant.com wrote:Oh, I forgot to mention that the configuration setting changes are also
>
> > +/*
> > + */
> > +static const struct config_enum_entry backup_block_compression_options[] = {
pending. I think we had a working consensus to use full_page_compression
as the name of the GUC. As I understand it, that'll accept an algorithm
name as an argument while we're still experimenting, but eventually once
we select an algorithm, it'll become just a boolean (and then we don't
need to put algorithm information into BkpBlock any more either).
-- Abhijit
В списке pgsql-hackers по дате отправления: