Re: [HACKERS] Cache Hash Index meta page.

On Tue, Feb 7, 2017 at 1:52 PM, Mithun Cy wrote: > On Tue, Feb 7, 2017 at 11:21 PM, Erik Rijkers wrote: >> On 2017-02-07 18:41, Robert Haas wrote: >>> >>> Committed with some changes (which I noted in the commit message). > > Thanks, Robert and all who have reviewed the patch and given their > va

Re: [HACKERS] Cache Hash Index meta page.

On Tue, Feb 7, 2017 at 11:21 PM, Erik Rijkers wrote: > On 2017-02-07 18:41, Robert Haas wrote: >> >> Committed with some changes (which I noted in the commit message). Thanks, Robert and all who have reviewed the patch and given their valuable comments. > This has caused a warning with gcc 6.20:

Re: [HACKERS] Cache Hash Index meta page.

On 2017-02-07 18:41, Robert Haas wrote: Committed with some changes (which I noted in the commit message). This has caused a warning with gcc 6.20: hashpage.c: In function ‘_hash_getcachedmetap’: hashpage.c:1245:20: warning: ‘cache’ may be used uninitialized in this function [-Wmaybe-uninitia

Re: [HACKERS] Cache Hash Index meta page.

On Wed, Feb 1, 2017 at 12:23 AM, Michael Paquier wrote: > On Sun, Jan 29, 2017 at 6:13 PM, Mithun Cy wrote: >>> HashMetaPage _hash_getcachedmetap(Relation rel, Buffer *metabuf, bool >>> force_refresh); >>> >>> If the cache is initialized and force_refresh is not true, then this >>> just returns t

Re: [HACKERS] Cache Hash Index meta page.

On Sun, Jan 29, 2017 at 6:13 PM, Mithun Cy wrote: >> HashMetaPage _hash_getcachedmetap(Relation rel, Buffer *metabuf, bool >> force_refresh); >> >> If the cache is initialized and force_refresh is not true, then this >> just returns the cached data, and the metabuf argument isn't used. >> Otherwis

Re: [HACKERS] Cache Hash Index meta page.

> HashMetaPage _hash_getcachedmetap(Relation rel, Buffer *metabuf, bool > force_refresh); > > If the cache is initialized and force_refresh is not true, then this > just returns the cached data, and the metabuf argument isn't used. > Otherwise, if *metabuf == InvalidBuffer, we set *metabuf to point

Re: [HACKERS] Cache Hash Index meta page.

On Thu, Jan 26, 2017 at 1:48 PM, Mithun Cy wrote: > _v11 API's was self-sustained one but it does not hold pins on the > metapage buffer. Whereas in _v12 we hold the pin for two consecutive > reads of metapage. I have taken your advice and producing 2 different > patches. Hmm. I think both of th

Re: [HACKERS] Cache Hash Index meta page.

On Tue, Jan 24, 2017 at 3:10 PM, Amit Kapila wrote: > 1. > @@ -505,26 +505,22 @@ hashbulkdelete(IndexVacuumInfo *info, > In the above flow, do we really need an updated metapage, can't we use > the cached one? We are already taking care of bucket split down in > that function. Yes, we can use t

Re: [HACKERS] Cache Hash Index meta page.

On Wed, Jan 18, 2017 at 11:51 AM, Mithun Cy wrote: > On Tue, Jan 17, 2017 at 10:07 AM, Amit Kapila wrote: > >> (b) Another somewhat bigger problem is that with this new change it >> won't retain the pin on meta page till the end which means we might >> need to perform an I/O again during operatio

Re: [HACKERS] Cache Hash Index meta page.

On Tue, Jan 17, 2017 at 10:07 AM, Amit Kapila wrote: > 1. > (a) I think you can retain the previous comment or modify it slightly. > Just removing the whole comment and replacing it with a single line > seems like a step backward. -- Fixed, Just modified to say it > (b) Another somewhat bigger p

Re: [HACKERS] Cache Hash Index meta page.

On Fri, Jan 13, 2017 at 9:58 AM, Mithun Cy wrote: > On Fri, Jan 6, 2017 at 11:43 AM, Amit Kapila wrote: Below are review comments on latest version of patch. 1. /* - * Read the metapage to fetch original bucket and tuple counts. Also, we - * keep a copy of the last-seen metapage so that we c

Re: [HACKERS] Cache Hash Index meta page.

On Wed, Jan 11, 2017 at 12:46 AM, Robert Haas wrote: > Can we adapt the ad-hoc caching logic in hashbulkdelete() to work with > this new logic? Or at least update the comments? I have introduced a new function _hash_getcachedmetap in patch 11 [1] with this hashbulkdelete() can use metapage cache

Re: [HACKERS] Cache Hash Index meta page.

On Fri, Jan 6, 2017 at 11:43 AM, Amit Kapila wrote: > Few more comments: > 1. > no need to two extra lines, one is sufficient and matches the nearby > coding pattern. -- Fixed. > 2. > Do you see anywhere else in the code the pattern of using @ symbol in > comments before function name? -- Fixed

Re: [HACKERS] Cache Hash Index meta page.

On Tue, Dec 27, 2016 at 3:06 AM, Mithun Cy wrote: > Thanks, just like _bt_getroot I am introducing a new function > _hash_getbucketbuf_from_hashkey() which will give the target bucket > buffer for the given hashkey. This will use the cached metapage > contents instead of reading meta page buffer i

Re: [HACKERS] Cache Hash Index meta page.

On Thu, Jan 5, 2017 at 11:38 AM, Mithun Cy wrote: > On Wed, Jan 4, 2017 at 5:21 PM, Mithun Cy wrote: > I have re-based the patch to fix one compilation warning > @_hash_doinsert where variable bucket was only used for Asserting but > was not declared about its purpose. > Few more comments: 1. }

Re: [HACKERS] Cache Hash Index meta page.

On Wed, Jan 4, 2017 at 5:21 PM, Mithun Cy wrote: I have re-based the patch to fix one compilation warning @_hash_doinsert where variable bucket was only used for Asserting but was not declared about its purpose. -- Thanks and Regards Mithun C Y EnterpriseDB: http://www.enterprisedb.com cache_h

Re: [HACKERS] Cache Hash Index meta page.

On Wed, Jan 4, 2017 at 4:19 PM, Mithun Cy wrote: > As part performance/space analysis of hash index on varchar data typevarchar > data type > with this patch, I have run some tests for same with modified pgbench.with > this patch, I have run some tests for same with modified pgbench. I forgot to

Re: [HACKERS] Cache Hash Index meta page.

On Tue, Jan 3, 2017 at 12:05 PM, Mithun Cy wrote: As part performance/space analysis of hash index on varchar data type with this patch, I have run some tests for same with modified pgbench. Modification includes: 1. Changed aid column of pg_accounts table from int to varchar(x) 2. To generate u

Re: [HACKERS] Cache Hash Index meta page.

Thanks Amit for detailed review, and pointing out various issues in the patch. I have tried to fix all of your comments as below. On Mon, Jan 2, 2017 at 11:29 AM, Amit Kapila wrote: > 1. > usage "into to .." in above comment seems to be wrong.usage "into to .." in > above comment seems to be wr

Re: [HACKERS] Cache Hash Index meta page.

On Tue, Dec 27, 2016 at 1:36 PM, Mithun Cy wrote: > On Thu, Dec 22, 2016 at 12:17 PM, Amit Kapila wrote: >> On Wed, Dec 21, 2016 at 9:26 PM, Robert Haas wrote: >>> On Tue, Dec 20, 2016 at 2:25 PM, Mithun Cy >>> wrote: -- I think if it is okay, I can document same for each member of

Re: [HACKERS] Cache Hash Index meta page.

On Tue, Dec 27, 2016 at 1:36 PM, Mithun Cy wrote: Oops, patch number should be 08 re-attaching same after renaming. -- Thanks and Regards Mithun C Y EnterpriseDB: http://www.enterprisedb.com diff --git a/src/backend/access/hash/hashinsert.c b/src/backend/access/hash/hashinsert.c index 46df589..c

Re: [HACKERS] Cache Hash Index meta page.

On Thu, Dec 22, 2016 at 12:17 PM, Amit Kapila wrote: > On Wed, Dec 21, 2016 at 9:26 PM, Robert Haas wrote: >> On Tue, Dec 20, 2016 at 2:25 PM, Mithun Cy >> wrote: >>> -- I think if it is okay, I can document same for each member of >>> HashMetaPageData whether to read from cached from meta pag

Re: [HACKERS] Cache Hash Index meta page.

On Wed, Dec 21, 2016 at 9:26 PM, Robert Haas wrote: > On Tue, Dec 20, 2016 at 2:25 PM, Mithun Cy wrote: >> -- I think if it is okay, I can document same for each member of >> HashMetaPageData whether to read from cached from meta page or directly from >> current meta page. Below briefly I have

Re: [HACKERS] Cache Hash Index meta page.

On Tue, Dec 20, 2016 at 2:25 PM, Mithun Cy wrote: > -- I think if it is okay, I can document same for each member of > HashMetaPageData whether to read from cached from meta page or directly from > current meta page. Below briefly I have commented for each member. If you > suggest I can go with

Re: [HACKERS] Cache Hash Index meta page.

On Tue, Dec 20, 2016 at 3:51 AM, Robert Haas wrote: > On Fri, Dec 16, 2016 at 5:16 AM, Mithun Cy > wrote: > >> Shouldn't _hash_doinsert() be using the cache, too >>> >> >> Yes, we have an opportunity there, added same in code. But one difference >> is at the end at-least once we need to read the

Re: [HACKERS] Cache Hash Index meta page.

On Fri, Dec 16, 2016 at 5:16 AM, Mithun Cy wrote: > Shouldn't _hash_doinsert() be using the cache, too >> > > Yes, we have an opportunity there, added same in code. But one difference > is at the end at-least once we need to read the meta page to split and/or > write. Performance improvement migh

Re: [HACKERS] Cache Hash Index meta page.

Thanks Robert, I have tried to address all of the comments, On Tue, Dec 6, 2016 at 2:20 AM, Robert Haas wrote: > > +bucket = _hash_hashkey2bucket(hashkey, metap->hashm_maxbucket, >metap->hashm_highmask, >metap-

Re: [HACKERS] Cache Hash Index meta page.

On Mon, Dec 5, 2016 at 2:58 PM, Mithun Cy wrote: > On Thu, Dec 1, 2016 at 8:10 PM, Jesper Pedersen < > jesper.peder...@redhat.com> wrote: > >As the concurrent hash index patch was committed in 6d46f4 this patch > needs a rebase. > > Thanks Jesper, > > Adding the rebased patch. > -bucket

Re: [HACKERS] Cache Hash Index meta page.

On Tue, Dec 6, 2016 at 1:28 AM, Mithun Cy wrote: > > *Clients * > > *Cache Meta Page patch * > > *Base code with amits changes* > > * %imp* > > 1 > > 17062.513102 > > 17218.353817 > > -0.9050848685 > > 8 > > 138525.808342 > > 128149.381759 > > 8.0971335488 > > 16 > > 2

Re: [HACKERS] Cache Hash Index meta page.

On Thu, Dec 1, 2016 at 8:10 PM, Jesper Pedersen wrote: >As the concurrent hash index patch was committed in 6d46f4 this patch needs a rebase. Thanks Jesper, Adding the rebased patch. I have re-run the pgbench readonly tests with below modification. "alter table pgbench_accounts drop constraint

Re: [HACKERS] Cache Hash Index meta page.

On 09/28/2016 11:55 AM, Mithun Cy wrote: On Tue, Sep 27, 2016 at 1:53 AM, Jeff Janes wrote: > I think that this needs to be updated again for v8 of concurrent and v5 of wal Adding the rebased patch over [1] + [2] As the concurrent hash index patch was committed in 6d46f4 this patch needs a

Re: [HACKERS] Cache Hash Index meta page.

On Tue, Oct 4, 2016 at 11:55 PM, Jeff Janes wrote: >Can you describe your benchmarking machine? Your benchmarking data went up to 128 clients. But how many cores does the machine have? Are >you testing how well it can use the resources it has, or how well it can deal with oversubscription of th

Re: [HACKERS] Cache Hash Index meta page.

On Fri, Jul 22, 2016 at 3:02 AM, Mithun Cy wrote: > I have created a patch to cache the meta page of Hash index in > backend-private memory. This is to save reading the meta page buffer every > time when we want to find the bucket page. In “_hash_first” call, we try > to read meta page buffer twi

Re: [HACKERS] Cache Hash Index meta page.

On Thu, Sep 29, 2016 at 12:55 AM, Mithun Cy wrote: > On Tue, Sep 27, 2016 at 1:53 AM, Jeff Janes wrote: > > I think that this needs to be updated again for v8 of concurrent and v5 > of wal > > Adding the rebased patch over [1] + [2] > > [1] Concurrent Hash index. > [2] Wal for hash index. Moved

Re: [HACKERS] Cache Hash Index meta page.

On Tue, Sep 27, 2016 at 1:53 AM, Jeff Janes wrote: > I think that this needs to be updated again for v8 of concurrent and v5 of wal Adding the rebased patch over [1] + [2] [1] Concurrent Hash index.

Re: [HACKERS] Cache Hash Index meta page.

On Tue, Sep 13, 2016 at 12:55 PM, Mithun Cy wrote: > On Thu, Sep 8, 2016 at 11:21 PM, Jesper Pedersen < > jesper.peder...@redhat.com> wrote: >> >> > For the archives, this patch conflicts with the WAL patch [1]. >> >> > [1] https://www.postgresql.org/message-id/CAA4eK1JS%2BSiRSQBzEFp >> nsSmxZKin

Re: [HACKERS] Cache Hash Index meta page.

On Thu, Sep 8, 2016 at 11:21 PM, Jesper Pedersen wrote: > > > For the archives, this patch conflicts with the WAL patch [1]. > > > [1] https://www.postgresql.org/message-id/CAA4eK1JS%2BSiRSQBzEFp > nsSmxZKingrRH7WNyWULJeEJSj1-%3D0w%40mail.gmail.com > Updated the patch it applies over Amit's concu

Re: [HACKERS] Cache Hash Index meta page.

On 09/05/2016 02:50 PM, Mithun Cy wrote: On Sep 2, 2016 7:38 PM, "Jesper Pedersen" wrote: Could you provide a rebased patch based on Amit's v5 ? Please find the the patch, based on Amit's V5. I have fixed following things 1. now in "_hash_first" we check if (opaque->hasho_prevblkno == Inval

Re: [HACKERS] Cache Hash Index meta page.

On Tue, Sep 6, 2016 at 12:20 AM, Mithun Cy wrote: > On Sep 2, 2016 7:38 PM, "Jesper Pedersen" > wrote: >> Could you provide a rebased patch based on Amit's v5 ? > > Please find the the patch, based on Amit's V5. > I think you want to say based on patch in the below mail: https://www.postgresql.o

Re: [HACKERS] Cache Hash Index meta page.

On Sep 2, 2016 7:38 PM, "Jesper Pedersen" wrote: > Could you provide a rebased patch based on Amit's v5 ? Please find the the patch, based on Amit's V5. I have fixed following things 1. now in "_hash_first" we check if (opaque->hasho_prevblkno == InvalidBlockNumber) to see if bucket is from old

Re: [HACKERS] Cache Hash Index meta page.

On 07/22/2016 06:02 AM, Mithun Cy wrote: I have created a patch to cache the meta page of Hash index in backend-private memory. This is to save reading the meta page buffer every time when we want to find the bucket page. In “_hash_first” call, we try to read meta page buffer twice just to make s

Re: [HACKERS] Cache Hash Index meta page.

On Thu, Aug 4, 2016 at 3:36 AM, Tom Lane wrote: > Jeff Janes writes: >> On Fri, Jul 22, 2016 at 3:02 AM, Mithun Cy >> wrote: >>> I have created a patch to cache the meta page of Hash index in >>> backend-private memory. This is to save reading the meta page buffer every >>> time when we want to

Re: [HACKERS] Cache Hash Index meta page.

Jeff Janes writes: > On Fri, Jul 22, 2016 at 3:02 AM, Mithun Cy wrote: >> I have created a patch to cache the meta page of Hash index in >> backend-private memory. This is to save reading the meta page buffer every >> time when we want to find the bucket page. In “_hash_first” call, we try >

Re: [HACKERS] Cache Hash Index meta page.

On Fri, Jul 22, 2016 at 3:02 AM, Mithun Cy wrote: > I have created a patch to cache the meta page of Hash index in > backend-private memory. This is to save reading the meta page buffer every > time when we want to find the bucket page. In “_hash_first” call, we try to > read meta page buffer twic

[HACKERS] Cache Hash Index meta page.

I have created a patch to cache the meta page of Hash index in backend-private memory. This is to save reading the meta page buffer every time when we want to find the bucket page. In “_hash_first” call, we try to read meta page buffer twice just to make sure bucket is not split after we found buck