Subject: Re: [HACKERS] Custom compression methods
To: Chris Travers <chris.travers@adjust.com>
Cc: Ildus Kurbangaliev <i.kurbangaliev@gmail.com>,
 David Steele <david@pgmasters.net>,
 Alexander Korotkov <a.korotkov@postgrespro.ru>,
 Dmitry Dolgov <9erthalion6@gmail.com>,
 PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
References: <20171201194859.le5hvnnrjzhxhm2t@alvherre.pgsql>
 <CAPpHfdtxo7VYd1hrrXAMkBZOk-ZPNhH=SFaJwaGedT5sZ34VrA@mail.gmail.com>
 <03c376ed-839f-35f4-5f03-35b21b47e9a2@postgrespro.ru>
 <CAPpHfduorgpFjQprDPXcUz3MYiau66z+fdrrepxhF+ZfDcy-aA@mail.gmail.com>
 <20180618173045.7f734aca@wp.localdomain>
 <20180702155624.542860bc@wp.localdomain>
 <CAPpHfduytTe0kxfDMZLWY=DXm0GBQKy-Km_gFcgpZ2UKAdHR5Q@mail.gmail.com>
 <20180906182713.502e1107@wp.localdomain>
 <20181101145437.1e00cb99@wp.localdomain>
 <CA+q6zcUTF97=Ao1C2b_0_6znzZX-eQtSB-KvTwZBV_X+e3StJw@mail.gmail.com>
 <20181203154332.7b1f4d6f@wp.localdomain>
 <20190228164415.62bd4107@ildus-work.localdomain>
 <53ab954a-bf44-2bff-fbe0-29f093899604@pgmasters.net>
 <CAPpHfdsQtby_G0HxPMrDA9G=swhC4o0sbm9hZ35A_uP8uw2TEQ@mail.gmail.com>
 <e686c67e-ec70-c081-825d-9a58a0a63595@pgmasters.net>
 <20190315125203.5da43edb@ildus-work.localdomain>
 <59ab2e0c-fd31-a063-28f8-9615dfe721ae@2ndquadrant.com>
 <CAN-RpxDFZUduYmOociaYHsnh3vw2E8wjsQ+Ht1xRvemp3H=6WA@mail.gmail.com>
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Message-ID: <87f7b3e6-8d48-654e-6ccc-571c99dce805@2ndquadrant.com>
Date: Tue, 19 Mar 2019 12:19:53 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.5.1
MIME-Version: 1.0
In-Reply-To: 
 <CAN-RpxDFZUduYmOociaYHsnh3vw2E8wjsQ+Ht1xRvemp3H=6WA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 3/19/19 10:59 AM, Chris Travers wrote:
> 
> 
> On Mon, Mar 18, 2019 at 11:09 PM Tomas Vondra
> <tomas.vondra@2ndquadrant.com <mailto:tomas.vondra@2ndquadrant.com>> wrote:
> 
> 
> 
>     On 3/15/19 12:52 PM, Ildus Kurbangaliev wrote:
>     > On Fri, 15 Mar 2019 14:07:14 +0400
>     > David Steele <david@pgmasters.net <mailto:david@pgmasters.net>> wrote:
>     >
>     >> On 3/7/19 11:50 AM, Alexander Korotkov wrote:
>     >>> On Thu, Mar 7, 2019 at 10:43 AM David Steele
>     <david@pgmasters.net <mailto:david@pgmasters.net>
>     >>> <mailto:david@pgmasters.net <mailto:david@pgmasters.net>>> wrote:
>     >>>
>     >>>     On 2/28/19 5:44 PM, Ildus Kurbangaliev wrote:
>     >>>
>     >>>      > there are another set of patches.
>     >>>      > Only rebased to current master.
>     >>>      >
>     >>>      > Also I will change status on commitfest to 'Needs review'.
>     >>>
>     >>>     This patch has seen periodic rebases but no code review that I
>     >>> can see since last January 2018.
>     >>>
>     >>>     As Andres noted in [1], I think that we need to decide if this
>     >>> is a feature that we want rather than just continuing to push it
>     >>> from CF to CF.
>     >>>
>     >>>
>     >>> Yes.  I took a look at code of this patch.  I think it's in pretty
>     >>> good shape.  But high level review/discussion is required.
>     >>
>     >> OK, but I think this patch can only be pushed one more time,
>     maximum,
>     >> before it should be rejected.
>     >>
>     >> Regards,
>     >
>     > Hi,
>     > in my opinion this patch is usually skipped not because it is not
>     > needed, but because of its size. It is not hard to maintain it until
>     > commiters will have time for it or I will get actual response that
>     > nobody is going to commit it.
>     >
> 
>     That may be one of the reasons, yes. But there are other reasons, which
>     I think may be playing a bigger role.
> 
>     There's one practical issue with how the patch is structured - the docs
>     and tests are in separate patches towards the end of the patch series,
>     which makes it impossible to commit the preceding parts. This needs to
>     change. Otherwise the patch size kills the patch as a whole.
> 
>     But there's a more important cost/benefit issue, I think. When I look at
>     patches as a committer, I naturally have to weight how much time I spend
>     on getting it in (and then dealing with fallout from bugs etc) vs. what
>     I get in return (measured in benefits for community, users). This patch
>     is pretty large and complex, so the "costs" are quite high, while the
>     benefits from the patch itself is the ability to pick between pg_lz and
>     zlib. Which is not great, and so people tend to pick other patches.
> 
>     Now, I understand there's a lot of potential benefits further down the
>     line, like column-level compression (which I think is the main goal
>     here). But that's not included in the patch, so the gains are somewhat
>     far in the future.
> 
> 
> Not discussing whether any particular committer should pick this up but
> I want to discuss an important use case we have at Adjust for this sort
> of patch.
> 
> The PostgreSQL compression strategy is something we find inadequate for
> at least one of our large deployments (a large debug log spanning
> 10PB+).  Our current solution is to set storage so that it does not
> compress and then run on ZFS to get compression speedups on spinning disks.
> 
> But running PostgreSQL on ZFS has some annoying costs because we have
> copy-on-write on copy-on-write, and when you add file fragmentation... I
> would really like to be able to get away from having to do ZFS as an
> underlying filesystem.  While we have good write throughput, read
> throughput is not as good as I would like.
> 
> An approach that would give us better row-level compression  would allow
> us to ditch the COW filesystem under PostgreSQL approach.
> 
> So I think the benefits are actually quite high particularly for those
> dealing with volume/variety problems where things like JSONB might be a
> go-to solution.  Similarly I could totally see having systems which
> handle large amounts of specialized text having extensions for dealing
> with these.
> 

Sure, I don't disagree - the proposed compression approach may be a big
win for some deployments further down the road, no doubt about it. But
as I said, it's unclear when we get there (or if the interesting stuff
will be in some sort of extension, which I don't oppose in principle).

> 
>     But hey, I think there are committers working for postgrespro, who might
>     have the motivation to get this over the line. Of course, assuming that
>     there are no serious objections to having this functionality or how it's
>     implemented ... But I don't think that was the case.
> 
> 
> While I am not currently able to speak for questions of how it is
> implemented, I can say with very little doubt that we would almost
> certainly use this functionality if it were there and I could see plenty
> of other cases where this would be a very appropriate direction for some
> other projects as well.
>
Well, I guess the best thing you can do to move this patch forward is to
actually try that on your real-world use case, and report your results
and possibly do a review of the patch.

IIRC there was an extension [1] leveraging this custom compression
interface for better jsonb compression, so perhaps that would work for
you (not sure if it's up to date with the current patch, though).

[1]
https://www.postgresql.org/message-id/20171130182009.1b492eb2%40wp.localdomain


regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services