On Fri, Dec 18, 2015 at 1:16 AM, Amit Kapila <amit.kapil...@gmail.com> wrote: > 1. At scale factor 300, there is gain of 11% at 128-client count and > 27% at 256 client count with Patch-1. At 4 clients, the performance with > Patch is 0.6% less (which might be a run-to-run variation or there could > be a small regression, but I think it is too less to be bothered about) > > 2. At scale factor 1000, there is no visible difference and there is some > at lower client count there is a <1% regression which could be due to > I/O bound nature of test. > > 3. On these runs, Patch-2 is mostly always worse than Patch-1, but > the difference between them is not significant.
Hmm, that's interesting. So the slots don't help. I was concerned that with only a single slot, you might have things moving quickly until you hit the point where you switch over to the next clog segment, and then you get a bad stall. It sounds like that either doesn't happen in practice, or more likely it does happen but the extra slot doesn't eliminate the stall because there's I/O at that point. Either way, it sounds like we can forget the slots idea for now. >> Some random comments: >> >> - TransactionGroupUpdateXidStatus could do just as well without >> add_proc_to_group. You could just say if (group_no >= NUM_GROUPS) >> break; instead. Also, I think you could combine the two if statements >> inside the loop. if (nextidx != INVALID_PGPROCNO && >> ProcGlobal->allProcs[nextidx].clogPage == proc->clogPage) break; or >> something like that. >> >> - memberXid and memberXidstatus are terrible names. Member of what? > > How about changing them to clogGroupMemberXid and > clogGroupMemberXidStatus? What we've currently got for group XID clearing for the ProcArray is clearXid, nextClearXidElem, and backendLatestXid. We should try to make these things consistent. Maybe rename those to procArrayGroupMember, procArrayGroupNext, procArrayGroupXid and then start all of these identifiers with clogGroup as you propose. >> That's going to be clear as mud to the next person looking at the >> definitiono f PGPROC. > > I understand that you don't like the naming convention, but using > such harsh language could sometimes hurt others. Sorry. If I am slightly frustrated here I think it is because this same point has been raised about three times now, by me and also by Andres, just with respect to this particular technique, and also on other patches. But you are right - that is no excuse for being rude. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
