Re: Patch for fast gin cache performance improvement

From: "Etsuro Fujita" <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
To: "'Ian Link'" <ian(at)ilink(dot)io>
Cc: "'pgsql-hackers'" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Patch for fast gin cache performance improvement
Date: 2013-10-10 08:01:44
Message-ID: 006401cec58e$f658b680$e30a2380$@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Ian Link wrote:
> > Although I asked this question, I've reconsidered about these
> > parameters, and it seems that these parameters not only make code
> > rather complex but are a little confusing to users. So I'd like to propose
> to introduce only one parameter:
> > fast_cache_size. While users that give weight to update performance
> > for the fast update technique set this parameter to a large value,
> > users that give weight not only to update performance but to search
> > performance set this parameter to a small value. What do you think about
> this?
> I think it makes sense to maintain this separation. If the user doesn't need
> a per-index setting, they don't have to use the parameter. Since the parameter
> is off by default, they don't even need to worry about it.
> There might as well be one parameter for users that don't need fine-grained
> control. We can document this and I don't think it will be confusing to the
> user.

OK, though I'd like to hear the opinion of others.

> > 4. In my understanding, the small value of
> > gin_fast_limit/fast_cache_size leads to the increase in GIN search
> > performance, which, however, leads to the decrease in GIN update
> > performance. Am I right? If so, I think the tradeoff should be noted in
> the documentation.
> I believe this is correct.
>
> > 5. The following documents in Chapter 57. GIN Indexes need to be
> > updated: * 57.3.1. GIN Fast Update Technique * 57.4. GIN Tips and
> > Tricks
> Sure, I can add something.
>
> > 6. I would like to see the results for the additional test cases
(tsvectors).
> I don't really have any good test cases for this available, and have very
limited
> time for postgres at the moment. Feel free to create a test case, but I don't
> believe I can at the moment. Sorry!

Unfortunately, I don't have much time to do such a thing, though I think we
should do that. (In addition, we should do another performance test to make
clear an influence of fast update performance from introducing these parameters,
which would be required to determine the default values of these parameters.)

> > 7. The commented-out elog() code should be removed.
> Sorry about that, I shouldn't have submitted the patch with those still there.
>
> I should have a new patch soonish, hopefully. Thanks for your feedback!

I think it is desirable that this patch should be resubmitted for the next
CommitFest for further review and testing mentioned above. So I'd like to mark
this patch as Returned with Feedback. Is it OK?

Thanks,

Best regards,
Etsuro Fujita

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dimitri Fontaine 2013-10-10 09:21:28 Re: CommitFest progress
Previous Message Andres Freund 2013-10-10 07:38:55 Re: Support for REINDEX CONCURRENTLY