Re: Speed up Clog Access by increasing CLOG buffers

On Mon, Feb 29, 2016 at 11:10 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Fri, Feb 26, 2016 at 11:37 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
wrote:
> > On Sat, Feb 27, 2016 at 10:03 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)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.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com