Re: [HACKERS] Cache Hash Index meta page.

2017-02-07 Thread Robert Haas
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

Re: [HACKERS] Cache Hash Index meta page.

2017-02-07 Thread Mithun Cy
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

Re: [HACKERS] Cache Hash Index meta page.

2017-02-07 Thread Erik Rijkers
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

Re: [HACKERS] Cache Hash Index meta page.

2017-02-07 Thread Robert Haas
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

Re: [HACKERS] Cache Hash Index meta page.

2017-01-31 Thread Michael Paquier
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

Re: [HACKERS] Cache Hash Index meta page.

2017-01-29 Thread Mithun Cy
> 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

Re: [HACKERS] Cache Hash Index meta page.

2017-01-27 Thread Robert Haas
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 >

Re: [HACKERS] Cache Hash Index meta page.

2017-01-26 Thread Mithun Cy
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

Re: [HACKERS] Cache Hash Index meta page.

2017-01-24 Thread Amit Kapila
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

Re: [HACKERS] Cache Hash Index meta page.

2017-01-17 Thread Mithun Cy
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)

Re: [HACKERS] Cache Hash Index meta page.

2017-01-16 Thread Amit Kapila
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

Re: [HACKERS] Cache Hash Index meta page.

2017-01-12 Thread Mithun Cy
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()

Re: [HACKERS] Cache Hash Index meta page.

2017-01-12 Thread Mithun Cy
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

Re: [HACKERS] Cache Hash Index meta page.

2017-01-10 Thread Robert Haas
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

Re: [HACKERS] Cache Hash Index meta page.

2017-01-05 Thread Amit Kapila
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

Re: [HACKERS] Cache Hash Index meta page.

2017-01-04 Thread Mithun Cy
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:

Re: [HACKERS] Cache Hash Index meta page.

2017-01-04 Thread Mithun Cy
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

Re: [HACKERS] Cache Hash Index meta page.

2017-01-04 Thread Mithun Cy
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

Re: [HACKERS] Cache Hash Index meta page.

2017-01-02 Thread Mithun Cy
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 >

Re: [HACKERS] Cache Hash Index meta page.

2017-01-01 Thread Amit Kapila
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

Re: [HACKERS] Cache Hash Index meta page.

2016-12-27 Thread Mithun Cy
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

Re: [HACKERS] Cache Hash Index meta page.

2016-12-27 Thread Mithun Cy
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

Re: [HACKERS] Cache Hash Index meta page.

2016-12-21 Thread Amit Kapila
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

Re: [HACKERS] Cache Hash Index meta page.

2016-12-21 Thread Robert Haas
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

Re: [HACKERS] Cache Hash Index meta page.

2016-12-20 Thread Mithun Cy
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

Re: [HACKERS] Cache Hash Index meta page.

2016-12-19 Thread Robert Haas
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.

Re: [HACKERS] Cache Hash Index meta page.

2016-12-16 Thread Mithun Cy
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, >

Re: [HACKERS] Cache Hash Index meta page.

2016-12-05 Thread Robert Haas
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

Re: [HACKERS] Cache Hash Index meta page.

2016-12-05 Thread Mithun Cy
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

Re: [HACKERS] Cache Hash Index meta page.

2016-12-05 Thread Mithun Cy
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

Re: [HACKERS] Cache Hash Index meta page.

2016-12-01 Thread Jesper Pedersen
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

Re: [HACKERS] Cache Hash Index meta page.

2016-10-05 Thread Mithun Cy
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

Re: [HACKERS] Cache Hash Index meta page.

2016-10-04 Thread Jeff Janes
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.

2016-10-02 Thread Michael Paquier
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]

Re: [HACKERS] Cache Hash Index meta page.

2016-09-28 Thread Mithun Cy
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.

2016-09-26 Thread Jeff Janes
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]

Re: [HACKERS] Cache Hash Index meta page.

2016-09-13 Thread Mithun Cy
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

Re: [HACKERS] Cache Hash Index meta page.

2016-09-08 Thread Jesper Pedersen
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

Re: [HACKERS] Cache Hash Index meta page.

2016-09-05 Thread Amit Kapila
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

Re: [HACKERS] Cache Hash Index meta page.

2016-09-05 Thread Mithun Cy
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)

Re: [HACKERS] Cache Hash Index meta page.

2016-09-02 Thread Jesper Pedersen
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

Re: [HACKERS] Cache Hash Index meta page.

2016-08-05 Thread Amit Kapila
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

Re: [HACKERS] Cache Hash Index meta page.

2016-08-03 Thread Tom Lane
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

Re: [HACKERS] Cache Hash Index meta page.

2016-08-03 Thread Jeff Janes
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