From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Chris Travers <chris.travers@adjust.com>
Cc: Ildus Kurbangaliev <i.kurbangaliev@gmail.com>
Cc: David Steele <david@pgmasters.net>
Cc: Alexander Korotkov <a.korotkov@postgrespro.ru>
Cc: Dmitry Dolgov <9erthalion6@gmail.com>
Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Subject: Re: [HACKERS] Custom compression methods
Date: Thu, 21 Mar 2019 20:59:56 +0100
Message-ID: <e02b7f01-ad9a-8b4c-609e-092a37d75926@2ndquadrant.com> (raw)
In-Reply-To: <CAN-RpxBuBvFFp-Ynq6y6ehwo1VKt-VtHgM0NTmxhQCXUiaqdjQ@mail.gmail.com>
References: <20171201194859.le5hvnnrjzhxhm2t@alvherre.pgsql>
<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>
<87f7b3e6-8d48-654e-6ccc-571c99dce805@2ndquadrant.com>
<CAN-RpxBuBvFFp-Ynq6y6ehwo1VKt-VtHgM0NTmxhQCXUiaqdjQ@mail.gmail.com>
On 3/19/19 4:44 PM, Chris Travers wrote:
>
>
> On Tue, Mar 19, 2019 at 12:19 PM Tomas Vondra
> <tomas.vondra@2ndquadrant.com <mailto:tomas.vondra@2ndquadrant.com>> wrote:
>
>
> On 3/19/19 10:59 AM, Chris Travers wrote:
> >
> >
> > 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).
>
>
> I would assume that if extensions are particularly stable and useful
> they could be moved into core.
>
> But I would also assume that at first, this area would be sufficiently
> experimental that folks (like us) would write our own extensions for it.
>
>
>
> >
> > 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.
>
>
> Yeah, I expect to do this within the next month or two.
>
>
>
> 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
>
> Yeah I will be looking at a couple different approaches here and
> reporting back. I don't expect it will be a full production workload but
> I do expect to be able to report on benchmarks in both storage and
> performance.
>
FWIW I was a bit curious how would that jsonb compression affect the
data set I'm using for testing jsonpath patches, so I spent a bit of
time getting it to work with master. It attached patch gets it to
compile, but unfortunately then it fails like this:
ERROR: jsonbd: worker has detached
It seems there's some bug in how sh_mq is used, but I don't have time
investigate that further.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
[text/x-patch] jsonbd-master-fix.patch (7.6K, ../e02b7f01-ad9a-8b4c-609e-092a37d75926@2ndquadrant.com/2-jsonbd-master-fix.patch)
download | inline diff:diff --git a/jsonbd.c b/jsonbd.cindex 511cfcf..9b86e1f 100644--- a/jsonbd.c+++ b/jsonbd.c@@ -766,11 +766,6 @@ jsonbd_cminitstate(Oid acoid, List *options)
return NULL;
}
-static void-jsonbd_cmdrop(Oid acoid)-{- /* TODO: if there is no compression options, remove the dictionary */-}
static struct varlena *
jsonbd_cmdecompress(CompressionAmOptions *cmoptions, const struct varlena *data)
@@ -863,7 +858,6 @@ jsonbd_compression_handler(PG_FUNCTION_ARGS)
CompressionAmRoutine *routine = makeNode(CompressionAmRoutine);
routine->cmcheck = jsonbd_cmcheck;
- routine->cmdrop = jsonbd_cmdrop; /* no drop behavior */
routine->cminitstate = jsonbd_cminitstate;
routine->cmcompress = jsonbd_cmcompress;
routine->cmdecompress = jsonbd_cmdecompress;
diff --git a/jsonbd_utils.c b/jsonbd_utils.cindex 3ab9347..029e2e3 100644--- a/jsonbd_utils.c+++ b/jsonbd_utils.c@@ -2,9 +2,11 @@
#include "jsonbd_utils.h"
#include "postgres.h"
+#include "access/genam.h"
#include "access/htup_details.h"
#include "access/xact.h"
#include "access/sysattr.h"
+#include "access/table.h"
#include "catalog/indexing.h"
#include "catalog/pg_extension.h"
#include "commands/extension.h"
@@ -13,22 +15,20 @@
#include "utils/fmgroids.h"
#include "utils/rel.h"
-#if PG_VERSION_NUM == 110000
struct shm_mq_alt
{
slock_t mq_mutex;
- PGPROC *mq_receiver; /* we need this one */- PGPROC *mq_sender; /* this one */- uint64 mq_bytes_read;- uint64 mq_bytes_written;+ PGPROC *mq_receiver;+ PGPROC *mq_sender;+ pg_atomic_uint64 mq_bytes_read;+ pg_atomic_uint64 mq_bytes_written;
Size mq_ring_size;
- bool mq_detached; /* and this one */+ bool mq_detached;+ uint8 mq_ring_offset;+ char mq_ring[FLEXIBLE_ARRAY_MEMBER];
/* in postgres version there are more attributes, but we don't need them */
};
-#else-#error "shm_mq struct in jsonbd is copied from PostgreSQL 11, please correct it according to your version"-#endif
/**
* Get 32-bit Murmur3 hash. Ported from qLibc library.
@@ -153,7 +153,7 @@ get_jsonbd_schema(void)
return InvalidOid; /* exit if pg_pathman does not exist */
ScanKeyInit(&entry[0],
- ObjectIdAttributeNumber,+ Anum_pg_extension_oid,
BTEqualStrategyNumber, F_OIDEQ,
ObjectIdGetDatum(ext_oid));
diff --git a/jsonbd_worker.c b/jsonbd_worker.cindex e9cb5b6..c59e102 100644--- a/jsonbd_worker.c+++ b/jsonbd_worker.c@@ -13,11 +13,11 @@
#include "access/xact.h"
#include "catalog/indexing.h"
#include "catalog/namespace.h"
-#include "catalog/pg_compression_opt.h"
#include "catalog/pg_extension.h"
#include "commands/extension.h"
#include "commands/dbcommands.h"
#include "executor/spi.h"
+#include "access/tableam.h"
#include "port/atomics.h"
#include "storage/ipc.h"
#include "storage/latch.h"
@@ -32,7 +32,6 @@
#include "utils/rel.h"
#include "utils/resowner.h"
#include "utils/snapmgr.h"
-#include "utils/tqual.h"
#include "utils/syscache.h"
static bool xact_started = false;
@@ -125,7 +124,7 @@ init_worker(dsm_segment *seg)
worker_args = (jsonbd_worker_args *) dsm_segment_address(seg);
/* Connect to our database */
- BackgroundWorkerInitializeConnectionByOid(worker_args->dboid, InvalidOid);+ BackgroundWorkerInitializeConnectionByOid(worker_args->dboid, InvalidOid, 0);
worker_state = shm_toc_lookup(toc, worker_args->worker_num, false);
worker_state->proc = MyProc;
@@ -205,8 +204,8 @@ jsonbd_get_key(Relation rel, Relation indrel, Oid cmoptoid, uint32 key_id)
IndexScanDesc scan;
ScanKeyData skey[2];
Datum key_datum;
- HeapTuple tup;
bool isNull;
+ TupleTableSlot *slot;
MemoryContext old_mcxt;
char *res;
@@ -224,19 +223,22 @@ jsonbd_get_key(Relation rel, Relation indrel, Oid cmoptoid, uint32 key_id)
scan = index_beginscan(rel, indrel, SnapshotAny, 2, 0);
index_rescan(scan, skey, 2, NULL, 0);
- tup = index_getnext(scan, ForwardScanDirection);- if (tup == NULL)+ slot = table_slot_create(rel, NULL);++ if (!index_getnext_slot(scan, ForwardScanDirection, slot))
elog(ERROR, "key not found for cmopt=%d and id=%d", cmoptoid, key_id);
- key_datum = heap_getattr(tup, JSONBD_DICTIONARY_REL_ATT_KEY,- RelationGetDescr(rel), &isNull);+ key_datum = slot_getattr(slot, JSONBD_DICTIONARY_REL_ATT_KEY, &isNull);
Assert(!isNull);
old_mcxt = MemoryContextSwitchTo(worker_context);
res = TextDatumGetCString(key_datum);
MemoryContextSwitchTo(old_mcxt);
+ ExecDropSingleTupleTableSlot(slot);+
index_endscan(scan);
+
return res;
}
@@ -308,7 +310,7 @@ jsonbd_get_key_id(Relation rel, Relation indrel, Oid cmoptoid, char *key)
{
IndexScanDesc scan;
ScanKeyData skey[2];
- HeapTuple tup;+ TupleTableSlot *slot;
bool isNull;
uint32 result = 0;
@@ -326,16 +328,18 @@ jsonbd_get_key_id(Relation rel, Relation indrel, Oid cmoptoid, char *key)
scan = index_beginscan(rel, indrel, SnapshotAny, 2, 0);
index_rescan(scan, skey, 2, NULL, 0);
- tup = index_getnext(scan, ForwardScanDirection);- if (tup != NULL)+ slot = table_slot_create(rel, NULL);++ if (index_getnext_slot(scan, ForwardScanDirection, slot))
{
- Datum dat = heap_getattr(tup, JSONBD_DICTIONARY_REL_ATT_ID,- RelationGetDescr(rel), &isNull);+ Datum dat = slot_getattr(slot, JSONBD_DICTIONARY_REL_ATT_ID, &isNull);
Assert(!isNull);
result = DatumGetInt32(dat);
}
index_endscan(scan);
+ ExecDropSingleTupleTableSlot(slot);+
return result;
}
@@ -547,7 +551,7 @@ jsonbd_launcher_main(Datum arg)
BackgroundWorkerUnblockSignals();
/* Init this launcher as backend so workers can notify it */
- InitPostgres(NULL, InvalidOid, NULL, InvalidOid, NULL);+ InitPostgres(NULL, InvalidOid, NULL, InvalidOid, NULL, false);
/* Create resource owner */
CurrentResourceOwner = ResourceOwnerCreate(NULL, "jsonbd_launcher_main");
@@ -821,10 +825,10 @@ jsonbd_register_worker(int worker_num, Oid dboid, int database_num)
worker.bgw_start_time = BgWorkerStart_ConsistentState;
worker.bgw_restart_time = 0;
worker.bgw_notify_pid = MyProcPid;
- memcpy(worker.bgw_library_name, "jsonbd", BGW_MAXLEN);- memcpy(worker.bgw_function_name, CppAsString(jsonbd_worker_main), BGW_MAXLEN);- snprintf(worker.bgw_name, BGW_MAXLEN, "jsonbd, worker %d, db: %d",- worker_num, dboid);+ strcpy(worker.bgw_library_name, "jsonbd");+ strcpy(worker.bgw_function_name, "jsonbd_worker_main");+ strcpy(worker.bgw_name, "jsonbd worker");+ strcpy(worker.bgw_type, "jsonbd worker");
worker.bgw_main_arg = UInt32GetDatum(dsm_segment_handle(seg));
/* Start dynamic worker */
@@ -835,11 +839,7 @@ jsonbd_register_worker(int worker_num, Oid dboid, int database_num)
}
/* Wait to be signalled. */
-#if PG_VERSION_NUM >= 100000- WaitLatch(&hdr->launcher_latch, WL_LATCH_SET, 0, PG_WAIT_EXTENSION);-#else- WaitLatch(&hdr->launcher_latch, WL_LATCH_SET, 0);-#endif+ WaitLatch(&hdr->launcher_latch, WL_LATCH_SET | WL_POSTMASTER_DEATH, 0, PG_WAIT_EXTENSION);
ResetLatch(&hdr->launcher_latch);
@@ -892,14 +892,14 @@ jsonbd_get_dictionary_relid(void)
Assert(relid != InvalidOid);
- rel = relation_open(relid, NoLock);+ rel = relation_open(relid, AccessShareLock);
indexes = RelationGetIndexList(rel);
Assert(list_length(indexes) == 2);
foreach(lc, indexes)
{
Oid indOid = lfirst_oid(lc);
- Relation indRel = index_open(indOid, NoLock);+ Relation indRel = index_open(indOid, AccessShareLock);
int attnum = indRel->rd_index->indkey.values[1];
if (attnum == JSONBD_DICTIONARY_REL_ATT_ID)
@@ -910,9 +910,9 @@ jsonbd_get_dictionary_relid(void)
jsonbd_keys_indoid = indOid;
}
- index_close(indRel, NoLock);+ index_close(indRel, AccessShareLock);
}
- relation_close(rel, NoLock);+ relation_close(rel, AccessShareLock);
}
finish_xact_command();
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tomas.vondra@2ndquadrant.com, chris.travers@adjust.com, i.kurbangaliev@gmail.com, david@pgmasters.net, a.korotkov@postgrespro.ru, 9erthalion6@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: [HACKERS] Custom compression methods
In-Reply-To: <e02b7f01-ad9a-8b4c-609e-092a37d75926@2ndquadrant.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox