Re: Speed up Clog Access by increasing CLOG buffers

On Mon, Feb 29, 2016 at 11:10 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Fri, Feb 26, 2016 at 11:37 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> > On Sat, Feb 27, 2016 at 10:03 AM, Amit Kapila <amit.kapila16@gmail.com>
> > wrote:
> >>
> >> Here, we can see that there is a gain of ~15% to ~38% at higher client
> >> count.
> >>
> >> The attached document (perf_write_clogcontrollock_data_v6.ods) contains
> >> data, mainly focussing on single client performance.  The data is for
> >> multiple runs on different machines, so I thought it is better to present in
> >> form of document rather than dumping everything in e-mail.  Do let me know
> >> if there is any confusion in understanding/interpreting the data.
> >
> > Forgot to mention that all these tests have been done by reverting
> > commit-ac1d794.
>
> OK, that seems better.  But I have a question: if we don't really need
> to make this optimization apply only when everything is on the same
> page, then why even try?  If we didn't try, we wouldn't need the
> all_trans_same_page flag, which would reduce the amount of code
> change.

I am not sure if I understood your question, do you want to know why at the first place transactions spanning more than one-page call the function TransactionIdSetPageStatus()?  If we want to avoid trying the transaction status update when they are on different page, then I think we need some

major changes in TransactionIdSetTreeStatus().

>  Would that hurt anything? Taking it even further, we could
> remove the check from TransactionGroupUpdateXidStatus too.  I'd be
> curious to know whether that set of changes would improve performance
> or regress it.  Or maybe it does nothing, in which case perhaps
> simpler is better.
>
> All things being equal, it's probably better if the cases where
> transactions from different pages get into the list together is
> something that is more or less expected rather than a
> once-in-a-blue-moon scenario - that way, if any bugs exist, we'll find
> them.  The downside of that is that we could increase latency for the
> leader that way - doing other work on the same page shouldn't hurt
> much but different pages is a bigger hit.  But that hit might be
> trivial enough not to be worth worrying about.
>

In my tests, the check related to same page in TransactionGroupUpdateXidStatus() doesn't impact performance in any way and I think the reason is that, it happens rarely that group contain multiple pages and even it occurs, there is hardly much impact.  So, I will remove that check and I think thats what you also want for now.

> +       /*
> +        * Now that we've released the lock, go back and wake everybody up.  We
> +        * don't do this under the lock so as to keep lock hold times to a
> +        * minimum.  The system calls we need to perform to wake other processes
> +        * up are probably much slower than the simple memory writes
> we did while
> +        * holding the lock.
> +        */
>
> This comment was true in the place that you cut-and-pasted it from,
> but it's not true here, since we potentially need to read from disk.
>

Okay, will change.