testing HS/SR - 1 vs 2 performance
in [PgSql]
|
Prev: pg_ctl stop -m immediate on the primary server inflatessequences
Next: [HACKERS] non-reproducible failure of random test on HEAD
From: Simon Riggs on 16 Apr 2010 08:00 On Fri, 2010-04-16 at 14:47 +0300, Heikki Linnakangas wrote: > Simon Riggs wrote: > > On Fri, 2010-04-16 at 11:29 +0300, Heikki Linnakangas wrote: > >> How does the attached patch look like? It's probably similar to what you > >> had in mind. > > > > It looks like a second version of what I'm working on and about to > > publish. I'll take that as a compliment! > > > > My patch is attached here also, for discussion. > > > > The two patches look same in their main parts, though I have quite a few > > extra tweaks in there, which you can read about in comments. > > Yeah. Yours seems a lot more complex with all those extra tweaks, I > would suggest to keep it simple. I did realize one bug in my patch: I > didn't handle xid wraparound correctly in the binary search, need to use > TransactionIdFollows instead of plan >. Almost done, yes, much simpler. I wrote a lot of that in the wee small hours last night, so the difference is amusing. And I spotted that bug, plus the off by one error also. Just rewritten all other parts, so no worries. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 16 Apr 2010 10:39 Heikki Linnakangas <heikki.linnakangas(a)enterprisedb.com> writes: > I didn't handle xid wraparound correctly in the binary search, need to use > TransactionIdFollows instead of plan >. I think you're outsmarting yourself there. A binary search will in fact *not work* with circular xid comparison (this is exactly why there's no btree opclass for XID). You need to use plain >, and make sure the array you're searching is ordered that way too. The other way might accidentally fail to malfunction if you only tested ranges of XIDs that weren't long enough to wrap around, but that doesn't make it right. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Simon Riggs on 16 Apr 2010 10:52 On Fri, 2010-04-16 at 10:39 -0400, Tom Lane wrote: > Heikki Linnakangas <heikki.linnakangas(a)enterprisedb.com> writes: > > I didn't handle xid wraparound correctly in the binary search, need to use > > TransactionIdFollows instead of plan >. > > I think you're outsmarting yourself there. A binary search will in fact > *not work* with circular xid comparison (this is exactly why there's no > btree opclass for XID). You need to use plain >, and make sure the > array you're searching is ordered that way too. The other way might > accidentally fail to malfunction if you only tested ranges of XIDs that > weren't long enough to wrap around, but that doesn't make it right. I don't understand the exact, please explain more. I'm not using bsearch() just a quick chop based upon xid comparison, which looks to me like it will work. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 16 Apr 2010 11:10 Simon Riggs <simon(a)2ndQuadrant.com> writes: > On Fri, 2010-04-16 at 10:39 -0400, Tom Lane wrote: >> I think you're outsmarting yourself there. A binary search will in fact >> *not work* with circular xid comparison (this is exactly why there's no >> btree opclass for XID). > I don't understand the exact, please explain more. > I'm not using bsearch() just a quick chop based upon xid comparison, > which looks to me like it will work. Implementing it yourself doesn't get you out of the fact that it won't work. Consider 1 2 3 ... 3000000000 ... 3999999999 and suppose that 3000000000 is the array's middle element. If you search for 100, your first probe will conclude that it is to the right of 3000000000, which is the wrong direction. Binary search, or indeed the entire concept that the array is "sorted" in the first place, depends on a transitive comparison operator. TransactionIdFollows does not satisfy the transitive law. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Simon Riggs on 16 Apr 2010 17:00
On Fri, 2010-04-16 at 13:00 +0100, Simon Riggs wrote: > Almost done Here's the finished patch. Internal changes only, no functional or user visible changes, so no docs, just detailed explanatory comments. Expectation is * performance no longer depends upon max_connections * recovery will be about same or slightly faster * snapshots should be about equivalent primary/standby Main changes are * lock free add to sorted array (equivalent to GetNewTransactionId) * bsearch for xid removal at commit/abort/TransactionIdIsInProgress * faster, more compact approach to snapshots, with self-trimming * some minor API changes to facilitate above * new code to ignore deletion failures only when !consistent Erik, Could you retest please? -- Simon Riggs www.2ndQuadrant.com |