Re: [HACKERS] Performance degradation in Bitmapscan (commit 75ae538bc3168bf44475240d4e0487ee2f3bb376)
On Fri, Jan 6, 2017 at 11:06 AM, Andres Freund wrote: > On 2017-01-06 11:01:32 -0500, Robert Haas wrote: >> On Fri, Jan 6, 2017 at 10:43 AM, Andres Freund wrote: >> > On 2016-12-16 09:34:31 -0800, Andres Freund wrote: >> >> > To fix his issue, we need something like your 0001. Are you going to >> >> > polish that up soon here? >> >> >> >> Yes. >> > >> > I've two versions of a fix for this. One of them basically increases the >> > "spread" of buckets when the density goes up too much. It does so by >> > basically shifting the bucket number to the left (e.g. only every second >> > bucket can be the "primary" bucket for a hash value). The other >> > basically just replaces the magic constants in my previous POC patch >> > with slightly better documented constants. For me the latter works just >> > as well as the former, even though aesthetically/theoretically the >> > former sounds better. I'm inclined to commit the latter, at least for >> > now. >> >> Did you intend to attach the patches? > > No, I hadn't. You're interested in the "spreading" version? I honestly have no opinion. If you're confident that you know what you want to do, it's fine with me if you just do it. If you want my opinion I probably need to see both patches and compare. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Performance degradation in Bitmapscan (commit 75ae538bc3168bf44475240d4e0487ee2f3bb376)
On 2017-01-06 11:01:32 -0500, Robert Haas wrote: > On Fri, Jan 6, 2017 at 10:43 AM, Andres Freund wrote: > > On 2016-12-16 09:34:31 -0800, Andres Freund wrote: > >> > To fix his issue, we need something like your 0001. Are you going to > >> > polish that up soon here? > >> > >> Yes. > > > > I've two versions of a fix for this. One of them basically increases the > > "spread" of buckets when the density goes up too much. It does so by > > basically shifting the bucket number to the left (e.g. only every second > > bucket can be the "primary" bucket for a hash value). The other > > basically just replaces the magic constants in my previous POC patch > > with slightly better documented constants. For me the latter works just > > as well as the former, even though aesthetically/theoretically the > > former sounds better. I'm inclined to commit the latter, at least for > > now. > > Did you intend to attach the patches? No, I hadn't. You're interested in the "spreading" version? Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Performance degradation in Bitmapscan (commit 75ae538bc3168bf44475240d4e0487ee2f3bb376)
On Fri, Jan 6, 2017 at 10:43 AM, Andres Freund wrote: > On 2016-12-16 09:34:31 -0800, Andres Freund wrote: >> > To fix his issue, we need something like your 0001. Are you going to >> > polish that up soon here? >> >> Yes. > > I've two versions of a fix for this. One of them basically increases the > "spread" of buckets when the density goes up too much. It does so by > basically shifting the bucket number to the left (e.g. only every second > bucket can be the "primary" bucket for a hash value). The other > basically just replaces the magic constants in my previous POC patch > with slightly better documented constants. For me the latter works just > as well as the former, even though aesthetically/theoretically the > former sounds better. I'm inclined to commit the latter, at least for > now. Did you intend to attach the patches? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Performance degradation in Bitmapscan (commit 75ae538bc3168bf44475240d4e0487ee2f3bb376)
On 2016-12-16 09:34:31 -0800, Andres Freund wrote: > > To fix his issue, we need something like your 0001. Are you going to > > polish that up soon here? > > Yes. I've two versions of a fix for this. One of them basically increases the "spread" of buckets when the density goes up too much. It does so by basically shifting the bucket number to the left (e.g. only every second bucket can be the "primary" bucket for a hash value). The other basically just replaces the magic constants in my previous POC patch with slightly better documented constants. For me the latter works just as well as the former, even though aesthetically/theoretically the former sounds better. I'm inclined to commit the latter, at least for now. Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Performance degradation in Bitmapscan (commit 75ae538bc3168bf44475240d4e0487ee2f3bb376)
On 2016-12-16 10:12:42 -0500, Robert Haas wrote: > On Wed, Dec 14, 2016 at 11:20 PM, Robert Haas wrote: > > I've got no problem with that at all, but I want to unbreak things > > more or less immediately and then you/we can further improve it later. > > Committed Thanks. Although "Fix drastic slowdown when ..." or so might have been a less confusing commit message ;) > , although I realize now that doesn't fix Dilip's problem, > only my (somewhat different) problem. Indeed. And the fix for Dilip's thing also fixes your issue, albeit what you committed still helps noticeably (even with the old hashtable). > To fix his issue, we need something like your 0001. Are you going to > polish that up soon here? Yes. Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Performance degradation in Bitmapscan (commit 75ae538bc3168bf44475240d4e0487ee2f3bb376)
On Wed, Dec 14, 2016 at 11:20 PM, Robert Haas wrote: > I've got no problem with that at all, but I want to unbreak things > more or less immediately and then you/we can further improve it later. Committed, although I realize now that doesn't fix Dilip's problem, only my (somewhat different) problem. To fix his issue, we need something like your 0001. Are you going to polish that up soon here? I kinda want to get the regressions we've introduced fixed here. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Performance degradation in Bitmapscan (commit 75ae538bc3168bf44475240d4e0487ee2f3bb376)
On Wed, Dec 14, 2016 at 10:08 PM, Andres Freund wrote: > On 2016-12-14 22:00:45 -0500, Robert Haas wrote: >> I took a look at Andres's patches today and saw that they can't really >> be applied as-is, because they "temporarily" change the master's >> ParallelWorkerNumber but have no provision for restoring it after an >> ERROR. > > Yea, that's not quite right. Although I'm not sure it really matters > that much, given that we're not continuing execution after an error. We > could just reset it at appropriate points - but that seems ugly. Yes. >> I think that approach isn't right anyway; it seems to me that >> what we want to do is set hash_iv based on we're in a Partial HashAgg, >> not whether we're in a parallel worker. For instance, if we're >> somehow in a nodeSubplan.c there's no need for this just because we >> happen to be in a worker -- I think. That led me to develop the >> attached patch. > > Hm, I'd rather have this be a more general solution - it doesn't seem > unlikely that other parts of the system want to know whether they're > currently executing a parallel portion of the plan. E.g. it really > should be forbidden to do modifications even in the parallel portion of > the plan the master executes. Right now that'd escape notice, which I > don't think is good. Actually, that wouldn't escape notice. You can test with IsInParallelMode(). That's entered before beginning execution of the plan at the outermost level - see ExecutePlan(). >> This may not be perfect, but it causes TPC-H Q18 to complete instead >> of hanging forever, so I'm going to commit it RSN unless there are >> loud objections combined with promising steps toward some alternative >> fix. It's been over a month since these problems were reported, and >> it is not good that the tree has been in a broken state for that >> entire time. > > Yea, sorry for that :(. Unfortunately the JIT stuff currently occupies a > large portion of my brain. > > I really hope to come up with a new version of the patch that does the > "smarter" expansion soon. I've got no problem with that at all, but I want to unbreak things more or less immediately and then you/we can further improve it later. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Performance degradation in Bitmapscan (commit 75ae538bc3168bf44475240d4e0487ee2f3bb376)
On 2016-12-14 22:00:45 -0500, Robert Haas wrote: > I took a look at Andres's patches today and saw that they can't really > be applied as-is, because they "temporarily" change the master's > ParallelWorkerNumber but have no provision for restoring it after an > ERROR. Yea, that's not quite right. Although I'm not sure it really matters that much, given that we're not continuing execution after an error. We could just reset it at appropriate points - but that seems ugly. > I think that approach isn't right anyway; it seems to me that > what we want to do is set hash_iv based on we're in a Partial HashAgg, > not whether we're in a parallel worker. For instance, if we're > somehow in a nodeSubplan.c there's no need for this just because we > happen to be in a worker -- I think. That led me to develop the > attached patch. Hm, I'd rather have this be a more general solution - it doesn't seem unlikely that other parts of the system want to know whether they're currently executing a parallel portion of the plan. E.g. it really should be forbidden to do modifications even in the parallel portion of the plan the master executes. Right now that'd escape notice, which I don't think is good. > This may not be perfect, but it causes TPC-H Q18 to complete instead > of hanging forever, so I'm going to commit it RSN unless there are > loud objections combined with promising steps toward some alternative > fix. It's been over a month since these problems were reported, and > it is not good that the tree has been in a broken state for that > entire time. Yea, sorry for that :(. Unfortunately the JIT stuff currently occupies a large portion of my brain. I really hope to come up with a new version of the patch that does the "smarter" expansion soon. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Performance degradation in Bitmapscan (commit 75ae538bc3168bf44475240d4e0487ee2f3bb376)
On Thu, Dec 8, 2016 at 5:23 PM, Robert Haas wrote: > On Wed, Nov 23, 2016 at 3:33 AM, Andres Freund wrote: >> On 2016-11-18 08:00:40 -0500, Robert Haas wrote: >>> On Tue, Nov 15, 2016 at 2:28 PM, Andres Freund wrote: >>> > I've a working fix for this, and for a similar issue Robert found. I'm >>> > still playing around with it, but basically the fix is to make the >>> > growth policy a bit more adaptive. >>> >>> Any chance you can post a patch soon? >> >> Here's my WIP series addressing this and related problems. With this >> we're again noticeably faster than the dynahash implementation, in both >> the case here, and the query you brought up over IM. >> >> This definitely needs some more TLC, but the general approach seems >> good. I particularly like that it apparently allows us to increase the >> default fillfactor without much downside according to my measurements. > > Are you going to commit something here? At least enough to make > Finalize HashAgg -> Gather -> Partial HashAgg terminate in finite > time? Because the fact that it doesn't really sucks. I took a look at Andres's patches today and saw that they can't really be applied as-is, because they "temporarily" change the master's ParallelWorkerNumber but have no provision for restoring it after an ERROR. I think that approach isn't right anyway; it seems to me that what we want to do is set hash_iv based on we're in a Partial HashAgg, not whether we're in a parallel worker. For instance, if we're somehow in a nodeSubplan.c there's no need for this just because we happen to be in a worker -- I think. That led me to develop the attached patch. This may not be perfect, but it causes TPC-H Q18 to complete instead of hanging forever, so I'm going to commit it RSN unless there are loud objections combined with promising steps toward some alternative fix. It's been over a month since these problems were reported, and it is not good that the tree has been in a broken state for that entire time. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company diff --git a/src/backend/executor/execGrouping.c b/src/backend/executor/execGrouping.c index 94cc59d..3149fbe 100644 --- a/src/backend/executor/execGrouping.c +++ b/src/backend/executor/execGrouping.c @@ -18,6 +18,8 @@ */ #include "postgres.h" +#include "access/hash.h" +#include "access/parallel.h" #include "executor/executor.h" #include "miscadmin.h" #include "utils/lsyscache.h" @@ -289,7 +291,8 @@ BuildTupleHashTable(int numCols, AttrNumber *keyColIdx, FmgrInfo *eqfunctions, FmgrInfo *hashfunctions, long nbuckets, Size additionalsize, - MemoryContext tablecxt, MemoryContext tempcxt) + MemoryContext tablecxt, MemoryContext tempcxt, + bool use_variable_hash_iv) { TupleHashTable hashtable; Size entrysize = sizeof(TupleHashEntryData) + additionalsize; @@ -314,6 +317,19 @@ BuildTupleHashTable(int numCols, AttrNumber *keyColIdx, hashtable->in_hash_funcs = NULL; hashtable->cur_eq_funcs = NULL; + /* + * If parallelism is in use, even if the master backend is performing the + * scan itself, we don't want to create the hashtable exactly the same way + * in all workers. As hashtables are iterated over in keyspace-order, + * doing so in all processes in the same way is likely to lead to + * "unbalanced" hashtables when the table size initially is + * underestimated. + */ + if (use_variable_hash_iv) + hashtable->hash_iv = hash_uint32(ParallelWorkerNumber); + else + hashtable->hash_iv = 0; + hashtable->hashtab = tuplehash_create(tablecxt, nbuckets); hashtable->hashtab->private_data = hashtable; @@ -450,7 +466,7 @@ TupleHashTableHash(struct tuplehash_hash *tb, const MinimalTuple tuple) TupleHashTable hashtable = (TupleHashTable) tb->private_data; int numCols = hashtable->numCols; AttrNumber *keyColIdx = hashtable->keyColIdx; - uint32 hashkey = 0; + uint32 hashkey = hashtable->hash_iv; TupleTableSlot *slot; FmgrInfo *hashfunctions; int i; diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index eefb3d6..a093862 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -1723,7 +1723,8 @@ build_hash_table(AggState *aggstate) node->numGroups, additionalsize, aggstate->aggcontexts[0]->ecxt_per_tuple_memory, - tmpmem); + tmpmem, + !DO_AGGSPLIT_SKIPFINAL(aggstate->aggsplit)); } /* diff --git a/src/backend/executor/nodeRecursiveunion.c b/src/backend/executor/nodeRecursiveunion.c index acded07..5b734c0 100644 --- a/src/backend/executor/nodeRecursiveunion.c +++ b/src/backend/executor/nodeRecursiveunion.c @@ -43,7 +43,8 @@ build_hash_table(RecursiveUnionState *rustate) node->numGroups, 0, rustate->tableContext, - rustate->tempContext); + rustate->tempContext, + false); } diff --git a/src/backend/executo
Re: [HACKERS] Performance degradation in Bitmapscan (commit 75ae538bc3168bf44475240d4e0487ee2f3bb376)
On Wed, Nov 23, 2016 at 3:33 AM, Andres Freund wrote: > On 2016-11-18 08:00:40 -0500, Robert Haas wrote: >> On Tue, Nov 15, 2016 at 2:28 PM, Andres Freund wrote: >> > I've a working fix for this, and for a similar issue Robert found. I'm >> > still playing around with it, but basically the fix is to make the >> > growth policy a bit more adaptive. >> >> Any chance you can post a patch soon? > > Here's my WIP series addressing this and related problems. With this > we're again noticeably faster than the dynahash implementation, in both > the case here, and the query you brought up over IM. > > This definitely needs some more TLC, but the general approach seems > good. I particularly like that it apparently allows us to increase the > default fillfactor without much downside according to my measurements. Are you going to commit something here? At least enough to make Finalize HashAgg -> Gather -> Partial HashAgg terminate in finite time? Because the fact that it doesn't really sucks. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Performance degradation in Bitmapscan (commit 75ae538bc3168bf44475240d4e0487ee2f3bb376)
On Wed, Nov 23, 2016 at 2:03 PM, Andres Freund wrote: > Here's my WIP series addressing this and related problems. With this > we're again noticeably faster than the dynahash implementation, in both > the case here, and the query you brought up over IM. > > This definitely needs some more TLC, but the general approach seems > good. I particularly like that it apparently allows us to increase the > default fillfactor without much downside according to my measurements. This patch fixes the performance problem which I have reported upthread. Time taken in Bitmap Index Scan is back to normal which was drastically improved by 75ae538bc3168bf44475240d4e0487ee2f3bb376 commit. -- - Limit (cost=1583130.59..1583130.60 rows=1 width=32) (actual time=14041.922..14041.923 rows=1 loops=1) -> Aggregate (cost=1583130.59..1583130.60 rows=1 width=32) (actual time=14041.921..14041.921 rows=1 loops=1) -> Bitmap Heap Scan on lineitem (cost=296012.30..1577117.95 rows=1202528 width=12) (actual time=1711.899..13332.892 rows=1190658 loops=1) Recheck Cond: ((l_shipdate >= '1995-01-01'::date) AND (l_shipdate < '1996-01-01 00:00:00'::timestamp without time zone) AND (l_discount >= 0.07 ) AND (l_discount <= 0.09) AND (l_quantity < '25'::numeric)) Rows Removed by Index Recheck: 27585320 Heap Blocks: exact=101349 lossy=580141 -> Bitmap Index Scan on idx_lineitem_shipdate (cost=0.00..295711.67 rows=1202528 width=0) (actual time=1689.478..1689.478 rows=1190658 loops= 1) Index Cond: ((l_shipdate >= '1995-01-01'::date) AND (l_shipdate < '1996-01-01 00:00:00'::timestamp without time zone) AND (l_discount >= 0.07) AND (l_discount <= 0.09) AND (l_quantity < '25'::numeric)) Planning time: 0.292 ms Execution time: 14041.968 ms (10 rows) revenue - 1784930119.2454 (1 row) -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Performance degradation in Bitmapscan (commit 75ae538bc3168bf44475240d4e0487ee2f3bb376)
On 2016-11-18 08:00:40 -0500, Robert Haas wrote: > On Tue, Nov 15, 2016 at 2:28 PM, Andres Freund wrote: > > I've a working fix for this, and for a similar issue Robert found. I'm > > still playing around with it, but basically the fix is to make the > > growth policy a bit more adaptive. > > Any chance you can post a patch soon? Here's my WIP series addressing this and related problems. With this we're again noticeably faster than the dynahash implementation, in both the case here, and the query you brought up over IM. This definitely needs some more TLC, but the general approach seems good. I particularly like that it apparently allows us to increase the default fillfactor without much downside according to my measurements. Greetings, Andres Freund >From efc603dd67a1c95f2d24844b86436ed6b7a28c80 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Wed, 23 Nov 2016 00:23:42 -0800 Subject: [PATCH 1/3] Resize simplehash.h table in case of long runs. This also allows to increase the default hashtable. --- src/include/lib/simplehash.h | 36 +--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/src/include/lib/simplehash.h b/src/include/lib/simplehash.h index 12aedbc..9522fa5 100644 --- a/src/include/lib/simplehash.h +++ b/src/include/lib/simplehash.h @@ -152,10 +152,12 @@ SH_SCOPE void SH_STAT(SH_TYPE *tb); #include "utils/memutils.h" -/* conservative fillfactor for a robin hood table, might want to adjust */ -#define SH_FILLFACTOR (0.8) +/* normal fillfactor, unless already close to maximum */ +#define SH_FILLFACTOR (0.9) /* increase fillfactor if we otherwise would error out */ -#define SH_MAX_FILLFACTOR (0.95) +#define SH_MAX_FILLFACTOR (0.98) +/* collision chain length at which we resize */ +#define SH_MAX_FILLFACTOR (0.98) /* max data array size,we allow up to PG_UINT32_MAX buckets, including 0 */ #define SH_MAX_SIZE (((uint64) PG_UINT32_MAX) + 1) @@ -550,6 +552,34 @@ SH_INSERT(SH_TYPE *tb, SH_KEY_TYPE key, bool *found) #endif entry->status = SH_STATUS_IN_USE; *found = false; + + /* + * To avoid negative consequences from overly imbalanced + * hashtables, grow the hashtable if collisions lead to large + * runs. The most likely cause of such imbalance is filling a + * (currently) small table, from a currently big one, in + * hash-table order. + * + * FIXME: compute boundaries in a more principled manner. + */ + if (unlikely(insertdist > 20 || + SH_DISTANCE_FROM_OPTIMAL(tb, curoptimal, emptyelem) > 1000)) + { +/* don't grow if the table would grow overly much */ +if (tb->members / ((double) tb->size) > 0.1) +{ + /* + * elog(WARNING, "clumping b, growing this thing"); + * SH_STAT(tb); + */ + /* + * Trigger a growth cycle next round, easier than resizing + * now. + */ + tb->grow_threshold = 0; +} + } + return entry; } -- 2.10.2 >From 150bc7fb1e9eacaee5d46852fa464e83e7930aca Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Tue, 22 Nov 2016 20:15:37 -0800 Subject: [PATCH 2/3] Signal when a master backend is doing parallel work. --- src/backend/access/transam/parallel.c | 14 -- src/backend/executor/nodeGather.c | 6 ++ src/include/access/parallel.h | 20 +++- 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c index 59dc394..c038ea1 100644 --- a/src/backend/access/transam/parallel.c +++ b/src/backend/access/transam/parallel.c @@ -88,12 +88,14 @@ typedef struct FixedParallelState } FixedParallelState; /* - * Our parallel worker number. We initialize this to -1, meaning that we are - * not a parallel worker. In parallel workers, it will be set to a value >= 0 - * and < the number of workers before any user code is invoked; each parallel - * worker will get a different parallel worker number. + * Our parallel worker number. We initialize this to PARALLEL_WORKER_MASTER, + * meaning that we are not a parallel worker. In parallel workers, it will be + * set to a value >= 0 and < the number of workers before any user code is + * invoked; each parallel worker will get a different parallel worker number. + * If the master backend performs parallel work, this will temporarily be set + * to PARALLEL_WORKER_MASTER_PARALLEL while doing so. */ -int ParallelWorkerNumber = -1; +int ParallelWorkerNumber = PARALLEL_WORKER_MASTER; /* Is there a parallel message pending which we need to receive? */ volatile bool ParallelMessagePending = false; @@ -955,7 +957,7 @@ ParallelWorkerMain(Datum main_arg) BackgroundWorkerUnblockSignals(); /* Determine and set our parallel worker number. */ - Assert(ParallelWorkerNumber == -1); + Assert(ParallelWorkerNumber == PARALLEL_WORKER_MASTER); memcpy(&ParallelWorkerNumber, MyBgworkerEntry->bgw_extra, sizeof(int)); /* Set up a memory context and resource ow
Re: [HACKERS] Performance degradation in Bitmapscan (commit 75ae538bc3168bf44475240d4e0487ee2f3bb376)
On Tue, Nov 15, 2016 at 2:28 PM, Andres Freund wrote: > I've a working fix for this, and for a similar issue Robert found. I'm > still playing around with it, but basically the fix is to make the > growth policy a bit more adaptive. Any chance you can post a patch soon? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Performance degradation in Bitmapscan (commit 75ae538bc3168bf44475240d4e0487ee2f3bb376)
On Wed, Nov 16, 2016 at 12:58 AM, Andres Freund wrote: > It's not really related to lossy pages, it's just that due to deletions > / insertions a lot more "shapes" of the hashtable are hit. Okay.. > > I suspect that this is with parallelism disabled? Without that the query > ends up using a parallel sequential scan for me. > It's with max_parallel_worker_per_gather=2, I always noticed that Q6 takes parallel seq scan only for max_parallel_worker_per_gather=4 or more.. > > I've a working fix for this, and for a similar issue Robert found. I'm > still playing around with it, but basically the fix is to make the > growth policy a bit more adaptive. Okay.. Thanks. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Performance degradation in Bitmapscan (commit 75ae538bc3168bf44475240d4e0487ee2f3bb376)
Hi Dilip, thanks for noticing that one. On 2016-11-09 21:17:22 +0530, Dilip Kumar wrote: > While testing bitmap performance, I have observed that some of the > TPCH queries taking huge time in BitmapIndexScan node, when there are > lossy pages. It's not really related to lossy pages, it's just that due to deletions / insertions a lot more "shapes" of the hashtable are hit. I suspect that this is with parallelism disabled? Without that the query ends up using a parallel sequential scan for me. I've a working fix for this, and for a similar issue Robert found. I'm still playing around with it, but basically the fix is to make the growth policy a bit more adaptive. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Performance degradation in Bitmapscan (commit 75ae538bc3168bf44475240d4e0487ee2f3bb376)
While testing bitmap performance, I have observed that some of the TPCH queries taking huge time in BitmapIndexScan node, when there are lossy pages. I suspected 75ae538bc3168bf44475240d4e0487ee2f3bb376 commit, because prior to that it used to take very less time. So I tested by reverting suspected commit and problem is solved. Here is explained analyze result for TPCH query 6 (scale factor 10) work_mem=10M shared_buffers=20GB machine under test: POWER, 4 socket machine ->On Head: postgres=# \i 6.explain.sql QUERY PLAN -- - Limit (cost=1585507.74..1585507.75 rows=1 width=32) (actual time=21626.467..21626.467 rows=1 loops=1) -> Aggregate (cost=1585507.74..1585507.75 rows=1 width=32) (actual time=21626.466..21626.466 rows=1 loops=1) -> Bitmap Heap Scan on lineitem (cost=299632.60..1579529.48 rows=1195652 width=12) (actual time=9204.770..20910.089 rows=1190658 loops=1) Recheck Cond: ((l_shipdate >= '1995-01-01'::date) AND (l_shipdate < '1996-01-01 00:00:00'::timestamp without time zone) AND (l_discount >= 0.07 ) AND (l_discount <= 0.09) AND (l_quantity < '25'::numeric)) Rows Removed by Index Recheck: 27584798 Heap Blocks: exact=101349 lossy=580141 -> Bitmap Index Scan on idx_lineitem_shipdate (cost=0.00..299333.68 rows=1195652 width=0) (actual time=9185.490..9185.490 rows=1190658 loops= 1) Index Cond: ((l_shipdate >= '1995-01-01'::date) AND (l_shipdate < '1996-01-01 00:00:00'::timestamp without time zone) AND (l_discount >= 0.07) AND (l_discount <= 0.09) AND (l_quantity < '25'::numeric)) Planning time: 0.675 ms Execution time: 21626.838 ms (10 rows) ->After reverting Commit: 75ae538bc3168bf44475240d4e0487ee2f3bb376 postgres=# \i 6.explain.sql QUERY PLAN -- - Limit (cost=1585507.74..1585507.75 rows=1 width=32) (actual time=12807.293..12807.294 rows=1 loops=1) -> Aggregate (cost=1585507.74..1585507.75 rows=1 width=32) (actual time=12807.291..12807.291 rows=1 loops=1) -> Bitmap Heap Scan on lineitem (cost=299632.60..1579529.48 rows=1195652 width=12) (actual time=1632.351..12131.552 rows=1190658 loops=1) Recheck Cond: ((l_shipdate >= '1995-01-01'::date) AND (l_shipdate < '1996-01-01 00:00:00'::timestamp without time zone) AND (l_discount >= 0.07 ) AND (l_discount <= 0.09) AND (l_quantity < '25'::numeric)) Rows Removed by Index Recheck: 28401743 Heap Blocks: exact=84860 lossy=596630 -> Bitmap Index Scan on idx_lineitem_shipdate (cost=0.00..299333.68 rows=1195652 width=0) (actual time=1613.166..1613.166 rows=1190658 loops= 1) Index Cond: ((l_shipdate >= '1995-01-01'::date) AND (l_shipdate < '1996-01-01 00:00:00'::timestamp without time zone) AND (l_discount >= 0.07) AND (l_discount <= 0.09) AND (l_quantity < '25'::numeric)) Planning time: 0.173 ms Execution time: 12807.380 ms (10 rows) >From above explain analyze result we can see that with commit 75ae538bc3168bf44475240d4e0487ee2f3bb376, Bitmap Index Scan node is way slower than without this commit. Perf result: Head + 13.12% 0.01% postgres postgres[.] tbm_lossify + 13.10%13.10% postgres postgres[.] tbm_mark_page_lossy + 12.84%12.82% postgres postgres[.] slot_deform_tuple +6.94% 0.00% postgres postgres[.] _bt_next +6.94% 0.02% postgres postgres[.] _bt_steppage +6.55% 0.05% postgres postgres[.] _bt_readpage +6.41% 1.00% postgres postgres[.] _bt_checkkeys After Reverting 75ae538bc3168bf44475240d4e0487ee2f3bb376: +0.71% 0.71% postgres postgres[.] cmp_var_common +0.62% 0.02% postgres postgres[.] tbm_lossify +0.62% 0.62% postgres postgres[.] AllocSetReset +0.60% 0.11% postgres [kernel.kallsyms] [k] sys_read +0.59% 0.10% postgres postgres[.] advance_transition_function I think in new hash implementation, delete from pagetable have severe performance issue. Note: If I set work_mem=100MB (no lossy page) then performance is fine. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers