Re: [HACKERS] SSI patch version 14

2011-02-10 Thread Heikki Linnakangas
On 09.02.2011 17:58, Kevin Grittner wrote: Dan Portsd...@csail.mit.edu wrote: I think for SerializableXidHash we should probably just initially allocate it at its maximum size. Then it'll match the PredXact list which is allocated in full upfront, and there's no risk of being able to allocate

Re: [HACKERS] SSI patch version 14

2011-02-10 Thread Andrew Dunstan
On 02/10/2011 05:09 AM, Heikki Linnakangas wrote: On 09.02.2011 17:58, Kevin Grittner wrote: Dan Portsd...@csail.mit.edu wrote: I think for SerializableXidHash we should probably just initially allocate it at its maximum size. Then it'll match the PredXact list which is allocated in full

Re: [HACKERS] SSI patch version 14

2011-02-09 Thread Heikki Linnakangas
On 09.02.2011 00:04, Kevin Grittner wrote: (1) When HTABs are created, there is the max_size, which is what the PredicateLockShmemSize function must use in its calculations, and the init_size, which is what will initially be allocated (and so, is probably what you see in the usage at the end of

Re: [HACKERS] SSI patch version 14

2011-02-09 Thread David Fetter
On Tue, Feb 08, 2011 at 09:09:48PM -0500, Robert Haas wrote: If we don't allocate all the memory up front, does that allow memory to be dynamically shared between different hash tables in shared memory? I'm thinking not, but... Frankly, I think this is an example of how our current shared

Re: [HACKERS] SSI patch version 14

2011-02-09 Thread Kevin Grittner
Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: (2) The predicate lock and lock target initialization code was initially copied and modified from the code for heavyweight locks. The heavyweight lock code adds 10% to the calculated maximum size. So I wound up doing that for

Re: [HACKERS] SSI patch version 14

2011-02-09 Thread Markus Wanner
On 02/09/2011 04:16 PM, David Fetter wrote: On Tue, Feb 08, 2011 at 09:09:48PM -0500, Robert Haas wrote: Frankly, I think this is an example of how our current shared memory model is a piece of garbage. What other model(s) might work better? Thread based, dynamically allocatable and

Re: [HACKERS] SSI patch version 14

2011-02-09 Thread Kevin Grittner
Dan Ports d...@csail.mit.edu wrote: I think for SerializableXidHash we should probably just initially allocate it at its maximum size. Then it'll match the PredXact list which is allocated in full upfront, and there's no risk of being able to allocate a transaction but not register its xid.

Re: [HACKERS] SSI patch version 14

2011-02-09 Thread Robert Haas
On Wed, Feb 9, 2011 at 10:38 AM, Markus Wanner mar...@bluegap.ch wrote: On 02/09/2011 04:16 PM, David Fetter wrote: On Tue, Feb 08, 2011 at 09:09:48PM -0500, Robert Haas wrote: Frankly, I think this is an example of how our current shared memory model is a piece of garbage. What other

Re: [HACKERS] SSI patch version 14

2011-02-09 Thread A.M.
On Feb 9, 2011, at 12:25 PM, Robert Haas wrote: On Wed, Feb 9, 2011 at 10:38 AM, Markus Wanner mar...@bluegap.ch wrote: On 02/09/2011 04:16 PM, David Fetter wrote: On Tue, Feb 08, 2011 at 09:09:48PM -0500, Robert Haas wrote: Frankly, I think this is an example of how our current shared

Re: [HACKERS] SSI patch version 14

2011-02-09 Thread Robert Haas
On Wed, Feb 9, 2011 at 2:16 PM, A.M. age...@themactionfaction.com wrote: On Feb 9, 2011, at 12:25 PM, Robert Haas wrote: On Wed, Feb 9, 2011 at 10:38 AM, Markus Wanner mar...@bluegap.ch wrote: On 02/09/2011 04:16 PM, David Fetter wrote: On Tue, Feb 08, 2011 at 09:09:48PM -0500, Robert Haas

Re: [HACKERS] SSI patch version 14

2011-02-09 Thread Markus Wanner
On 02/09/2011 06:25 PM, Robert Haas wrote: On Wed, Feb 9, 2011 at 10:38 AM, Markus Wanner mar...@bluegap.ch wrote: Thread based, dynamically allocatable and resizeable shared memory, as most other projects and developers use, for example. I didn't mean to say we should switch to that model.

Re: [HACKERS] SSI patch version 14

2011-02-09 Thread Robert Haas
On Wed, Feb 9, 2011 at 2:51 PM, Markus Wanner mar...@bluegap.ch wrote: On 02/09/2011 06:25 PM, Robert Haas wrote: On Wed, Feb 9, 2011 at 10:38 AM, Markus Wanner mar...@bluegap.ch wrote: Thread based, dynamically allocatable and resizeable shared memory, as most other projects and developers

Re: [HACKERS] SSI patch version 14

2011-02-08 Thread Kevin Grittner
Heikki Linnakangas wrote: Ok, committed. I see that at least three BuildFarm critters don't have UINT64_MAX defined. Not sure why coypu is running out of connections. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:

Re: [HACKERS] SSI patch version 14

2011-02-08 Thread Heikki Linnakangas
On 08.02.2011 10:43, Kevin Grittner wrote: Heikki Linnakangas wrote: Ok, committed. I see that at least three BuildFarm critters don't have UINT64_MAX defined. I guess we'll have to just #define it ourselves. Or could we just pick another magic value, do we actually rely on

Re: [HACKERS] SSI patch version 14

2011-02-08 Thread Bruce Momjian
Kevin Grittner wrote: Heikki Linnakangas wrote: Ok, committed. I see that at least three BuildFarm critters don't have UINT64_MAX defined. Not sure why coypu is running out of connections. Yes, I am seeing that failures here too on my BSD machine. -- Bruce Momjian

Re: [HACKERS] SSI patch version 14

2011-02-08 Thread Kevin Grittner
Heikki Linnakangas wrote: On 08.02.2011 10:43, Kevin Grittner wrote: I see that at least three BuildFarm critters don't have UINT64_MAX defined. I guess we'll have to just #define it ourselves. Or could we just pick another magic value, do we actually rely on InvalidSerCommitSeqno

Re: [HACKERS] SSI patch version 14

2011-02-08 Thread Dan Ports
On Tue, Feb 08, 2011 at 11:25:34AM +0200, Heikki Linnakangas wrote: On 08.02.2011 10:43, Kevin Grittner wrote: I see that at least three BuildFarm critters don't have UINT64_MAX defined. I guess we'll have to just #define it ourselves. Or could we just pick another magic value, do we

Re: [HACKERS] SSI patch version 14

2011-02-08 Thread Kevin Grittner
Dan Ports d...@csail.mit.edu wrote: On Tue, Feb 08, 2011 at 11:25:34AM +0200, Heikki Linnakangas wrote: On 08.02.2011 10:43, Kevin Grittner wrote: I see that at least three BuildFarm critters don't have UINT64_MAX defined. I guess we'll have to just #define it ourselves. Or could we just

Re: [HACKERS] SSI patch version 14

2011-02-08 Thread Kevin Grittner
I wrote: The multiplier of 10 PredXactList structures per connection is kind of arbitrary. It affects the point at which information is pushed to the lossy summary, so any number from 2 up will work correctly; it's a matter of performance and false positive rate. We might want to put that

Re: [HACKERS] SSI patch version 14

2011-02-08 Thread Heikki Linnakangas
On 08.02.2011 17:50, Kevin Grittner wrote: Attached is something which will work. Whether people prefer this or a definition of UINT64_MAX in some header file (if it's missing) doesn't matter much to me. At any rate, if someone commits this one-liner, three critters should go back to green.

Re: [HACKERS] SSI patch version 14

2011-02-08 Thread Dan Ports
On Tue, Feb 08, 2011 at 10:14:44AM -0600, Kevin Grittner wrote: I do have some concern that if this defaults to too low a number, those who try SSI without bumping it and restarting the postmaster will not like the performance under load very much. SSI performance would not be affected by a

Re: [HACKERS] SSI patch version 14

2011-02-08 Thread Heikki Linnakangas
On 08.02.2011 18:14, Kevin Grittner wrote: I wrote: The multiplier of 10 PredXactList structures per connection is kind of arbitrary. It affects the point at which information is pushed to the lossy summary, so any number from 2 up will work correctly; it's a matter of performance and false

Re: [HACKERS] SSI patch version 14

2011-02-08 Thread Kevin Grittner
Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: Taking RWConflictPool into account in PredicateLockShmemSize() fixes the underestimate, but makes the overestimate correspondingly larger. I've never compared the actual and estimated shmem sizes of other parts of the backend,

Re: [HACKERS] SSI patch version 14

2011-02-08 Thread Dan Ports
One other nit re. the predicate lock table size GUCs: the out-of-memory case in RegisterPredicateLockingXid (predicate.c:1592 in my tree) gives the hint to increase max_predicate_locks_per_transaction. I don't think that's correct, since that GUC isn't used to size SerializableXidHash. In fact,

Re: [HACKERS] SSI patch version 14

2011-02-08 Thread Kevin Grittner
Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: LOG: shmemsize 3153112 LOG: actual 2339864 Which is a pretty big overestimate, percentage-wise. Taking RWConflictPool into account in PredicateLockShmemSize() fixes the underestimate, but makes the overestimate

Re: [HACKERS] SSI patch version 14

2011-02-08 Thread Dan Ports
On Tue, Feb 08, 2011 at 04:04:39PM -0600, Kevin Grittner wrote: (2) The predicate lock and lock target initialization code was initially copied and modified from the code for heavyweight locks. The heavyweight lock code adds 10% to the calculated maximum size. So I wound up doing that for

Re: [HACKERS] SSI patch version 14

2011-02-08 Thread Robert Haas
On Tue, Feb 8, 2011 at 7:23 PM, Dan Ports d...@csail.mit.edu wrote: On Tue, Feb 08, 2011 at 04:04:39PM -0600, Kevin Grittner wrote: (2)  The predicate lock and lock target initialization code was initially copied and modified from the code for heavyweight locks. The heavyweight lock code adds

Re: [HACKERS] SSI patch version 14

2011-02-08 Thread Dan Ports
On Tue, Feb 08, 2011 at 09:09:48PM -0500, Robert Haas wrote: No way to fail is a tall order. Well, no way to fail due to running out of shared memory in RegisterPredicateLock/RegisterPredicateLockingXid, but that doesn't have quite the same ring to it... If we don't allocate all the memory up

Re: [HACKERS] SSI patch version 14

2011-02-07 Thread Heikki Linnakangas
On 05.02.2011 21:43, Kevin Grittner wrote: Kevin Grittner wrote: So now that I'm sure we actually do need code there, I'll add it. In working on this I noticed the apparent need to move two calls to PredicateLockTuple a little bit to keep them inside the buffer lock. Without at least a

Re: [HACKERS] SSI patch version 14

2011-02-07 Thread Kevin Grittner
Kevin Grittner kgri...@wicourts.gov wrote: Kevin Grittner wrote: Jeff Davis wrote: What does PredicateLockTuple do that needs a share lock? Does a pin suffice? If one process is reading a tuple and another is writing it (e.g., UPDATE or DELETE) the concern is that we need to be able

Re: [HACKERS] SSI patch version 14

2011-02-07 Thread Heikki Linnakangas
On 06.02.2011 20:30, Kevin Grittner wrote: Kevin Grittner wrote: I'm working on it now, and hope to have it all settled down today. Done and pushed to the git repo. Branch serializable on: git://git.postgresql.org/git/users/kgrittn/postgres.git Heikki: I'm back to not having any

Re: [HACKERS] SSI patch version 14

2011-02-07 Thread Magnus Hagander
On Mon, Feb 7, 2011 at 23:14, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: On 06.02.2011 20:30, Kevin Grittner wrote: Kevin Grittner  wrote: I'm working on it now, and hope to have it all settled down today. Done and pushed to the git repo.  Branch serializable on:

Re: [HACKERS] SSI patch version 14

2011-02-07 Thread Robert Haas
On Mon, Feb 7, 2011 at 5:23 PM, Magnus Hagander mag...@hagander.net wrote: On Mon, Feb 7, 2011 at 23:14, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: On 06.02.2011 20:30, Kevin Grittner wrote: Kevin Grittner  wrote: I'm working on it now, and hope to have it all settled

Re: [HACKERS] SSI patch version 14

2011-02-07 Thread Kevin Grittner
Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: Ok, committed. Thanks for that, and for all the suggestions along the way! Thanks also to Joe Conway for the review and partial commit in the first CF for this release; to Jeff Davis for the reviews, pointing out areas which

Re: [HACKERS] SSI patch version 14

2011-02-06 Thread Kevin Grittner
I wrote: First cut [of having predicate locks linked to later row versions for conflict detection purposes] It passes all the usual regression tests, the new isolation tests, and the example posted earlier in the thread of a test case which was allowing an anomaly. (That is to say, the

Re: [HACKERS] SSI patch version 14

2011-02-06 Thread Kevin Grittner
Kevin Grittner wrote: I'm working on it now, and hope to have it all settled down today. Done and pushed to the git repo. Branch serializable on: git://git.postgresql.org/git/users/kgrittn/postgres.git Heikki: I'm back to not having any outstanding work on the patch. Does anyone want

Re: [HACKERS] SSI patch version 14

2011-02-06 Thread Jeff Davis
On Sat, 2011-02-05 at 14:43 -0600, Kevin Grittner wrote: Kevin Grittner wrote: So now that I'm sure we actually do need code there, I'll add it. In working on this I noticed the apparent need to move two calls to PredicateLockTuple a little bit to keep them inside the buffer lock.

Re: [HACKERS] SSI patch version 14

2011-02-06 Thread Kevin Grittner
Jeff Davis wrote: On Sat, 2011-02-05 at 14:43 -0600, Kevin Grittner wrote: In working on this I noticed the apparent need to move two calls to PredicateLockTuple a little bit to keep them inside the buffer lock. Without at least a share lock on the buffer, it seems that here is a window

Re: [HACKERS] SSI patch version 14

2011-02-06 Thread Kevin Grittner
Kevin Grittner wrote: Jeff Davis wrote: What does PredicateLockTuple do that needs a share lock? Does a pin suffice? If one process is reading a tuple and another is writing it (e.g., UPDATE or DELETE) the concern is that we need to be able to guarantee that either the predicate lock

Re: [HACKERS] SSI patch version 14

2011-02-05 Thread Kevin Grittner
Heikki Linnakangas wrote: On 02.02.2011 19:36, Kevin Grittner wrote: Then there's one still lurking outside the new predicate* files, in heapam.c. It's about 475 lines into the heap_update function (25 lines from the bottom). In reviewing where we needed to acquire predicate locks, this

Re: [HACKERS] SSI patch version 14

2011-02-05 Thread Kevin Grittner
Kevin Grittner wrote: So now that I'm sure we actually do need code there, I'll add it. In working on this I noticed the apparent need to move two calls to PredicateLockTuple a little bit to keep them inside the buffer lock. Without at least a share lock on the buffer, it seems that here is

Re: [HACKERS] SSI patch version 14

2011-02-05 Thread Kevin Grittner
Kevin Grittner wrote: So now that I'm sure we actually do need code there, I'll add it. Hmmm... First cut at this here: http://git.postgresql.org/gitweb?p=users/kgrittn/postgres.git;a=commitdiff;h=3ccedeb451ee74ee78ef70735782f3550b621eeb It passes all the usual regression tests, the new

Re: [HACKERS] SSI patch version 14

2011-02-04 Thread Heikki Linnakangas
On 02.02.2011 19:36, Kevin Grittner wrote: I really appreciate you putting this testing framework together. This is clearly the right way to do it, although we also clearly don't want this test in the check target at the root directory, because of the run time. It would be nice to get some

Re: [HACKERS] SSI patch version 14

2011-02-04 Thread Kevin Grittner
Heikki Linnakangas wrote: On 02.02.2011 19:36, Kevin Grittner wrote: I really appreciate you putting this testing framework together. This is clearly the right way to do it, although we also clearly don't want this test in the check target at the root directory, because of the run time.

Re: [HACKERS] SSI patch version 14

2011-02-04 Thread Heikki Linnakangas
On 04.02.2011 14:30, Kevin Grittner wrote: Heikki Linnakangas wrote: The assertion in RecordFreeIndexPage() is a reasonable sanity check, but I'm inclined to move it to the caller instead: I don't think the FSM should need to access predicate lock manager, even for an assertion. OK. So it

Re: [HACKERS] SSI patch version 14

2011-02-04 Thread Kevin Grittner
Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: On 04.02.2011 14:30, Kevin Grittner wrote: OK. So it looks like right now it will move to btvacuumpage(), right before the call to RecordFreeIndexPage(), and we will need to add it to one spot each in the GiST and GIN AMs, when

Re: [HACKERS] SSI patch version 14

2011-02-02 Thread Heikki Linnakangas
On 26.01.2011 13:36, Simon Riggs wrote: I looked at the patch to begin a review and immediately saw dtest. There's no docs to explain what it is, but a few comments fill me in a little more. Can we document that please? And/or explain why its an essential part of this commit? Could we keep it

Re: [HACKERS] SSI patch version 14

2011-02-02 Thread Markus Wanner
Heikki, On 02/02/2011 11:20 AM, Heikki Linnakangas wrote: I couldn't get it to run on my basic Debian system, clearly I'm doing something wrong, but it will be even harder for people on more exotic platforms. On Debian you only need 'python-twisted' and the dtester sources [1], IIRC. What

Re: [HACKERS] SSI patch version 14

2011-02-02 Thread Heikki Linnakangas
On 02.02.2011 12:20, Heikki Linnakangas wrote: On 26.01.2011 13:36, Simon Riggs wrote: I looked at the patch to begin a review and immediately saw dtest. There's no docs to explain what it is, but a few comments fill me in a little more. Can we document that please? And/or explain why its an

Re: [HACKERS] SSI patch version 14

2011-02-02 Thread Heikki Linnakangas
On 02.02.2011 14:48, Markus Wanner wrote: Heikki, On 02/02/2011 11:20 AM, Heikki Linnakangas wrote: I couldn't get it to run on my basic Debian system, clearly I'm doing something wrong, but it will be even harder for people on more exotic platforms. On Debian you only need 'python-twisted'

Re: [HACKERS] SSI patch version 14

2011-02-02 Thread Markus Wanner
Heikki, On 02/02/2011 02:04 PM, Heikki Linnakangas wrote: TypeError: __init__() got an unexpected keyword argument 'showTimingInfo' make: *** [dcheck] Virhe 1 At that point I had no idea what's wrong. Hm.. looks like Kevin already uses the latest dtester code from git. You can either do

Re: [HACKERS] SSI patch version 14

2011-02-02 Thread Kevin Grittner
Heikki Linnakangas wrote: On 02.02.2011 12:20, Heikki Linnakangas wrote: The tests don't seem very complicated, and they don't seem to depend much on the dtester features. How hard would it be to rewrite the test engine in C or perl? I'm thinking of defining each test in a simple text

Re: [HACKERS] SSI patch version 14

2011-02-02 Thread Heikki Linnakangas
On 02.02.2011 16:11, Kevin Grittner wrote: Heikki Linnakangas wrote: On 02.02.2011 12:20, Heikki Linnakangas wrote: The tests don't seem very complicated, and they don't seem to depend much on the dtester features. How hard would it be to rewrite the test engine in C or perl? I'm thinking

Re: [HACKERS] SSI patch version 14

2011-02-02 Thread Kevin Grittner
Heikki Linnakangas wrote: I converted the next test, receipt-report, grab that from my git repository first. Will do. It produces over two hundred permutations, so it's going to be a bit tedious to check the output for that, but I think it's still acceptable so that we don't need more

Re: [HACKERS] SSI patch version 14

2011-02-02 Thread Heikki Linnakangas
On 02.02.2011 16:27, Kevin Grittner wrote: Heikki Linnakangas wrote: It produces over two hundred permutations, so it's going to be a bit tedious to check the output for that, but I think it's still acceptable so that we don't need more complicated rules for what is accepted. IOW, we can just

Re: [HACKERS] SSI patch version 14

2011-02-02 Thread Kevin Grittner
Heikki Linnakangas wrote: I converted the next test, receipt-report, grab that from my git repository first. It produces over two hundred permutations, so it's going to be a bit tedious to check the output for that, but I think it's still acceptable so that we don't need more complicated

Re: [HACKERS] SSI patch version 14

2011-02-02 Thread Heikki Linnakangas
On 02.02.2011 17:52, Kevin Grittner wrote: I found two problems with this, and I'm not sure how to handle either. If we can figure out an approach, I'm happy to work on it. (1) We don't have much in the way of detail yet. I put a different detail message on each, so that there is more

Re: [HACKERS] SSI patch version 14

2011-02-02 Thread Kevin Grittner
Heikki Linnakangas wrote: Suppressing detail seems easiest. AFAICS all the variable parts are in errdetail. I pushed that with some work on the tests. If you could review the changes to isolationtester.c to confirm that they look sane, I'd appreciate it.

Re: [HACKERS] SSI patch version 14

2011-02-02 Thread Heikki Linnakangas
On 02.02.2011 19:36, Kevin Grittner wrote: Heikki Linnakangas wrote: Suppressing detail seems easiest. AFAICS all the variable parts are in errdetail. I pushed that with some work on the tests. If you could review the changes to isolationtester.c to confirm that they look sane, I'd

Re: [HACKERS] SSI patch version 14

2011-02-02 Thread Kevin Grittner
Heikki Linnakangas wrote: A comment would be in order to explain why we only print the primary message. Done: http://git.postgresql.org/gitweb?p=users/kgrittn/postgres.git;a=commitdiff;h=ddeea22db06ed763b39f716b86db248008a3aa92 -Kevin -- Sent via pgsql-hackers mailing list

Re: [HACKERS] SSI patch version 14

2011-02-02 Thread Kevin Grittner
Heikki Linnakangas wrote: On 02.02.2011 19:36, Kevin Grittner wrote: If that part is right, it shouldn't take me very long to finish the specs and capture the expected results. Looks good. First cut on the rest of it pushed. I'll want to go over it again to confirm, but I think all

Re: [HACKERS] SSI patch version 14

2011-02-01 Thread Jeff Davis
On Mon, 2011-01-31 at 17:55 -0600, Kevin Grittner wrote: http://git.postgresql.org/gitweb?p=users/kgrittn/postgres.git;a=commitdiff;h=6360b0d4ca88c09cf590a75409cd29831afff58b With confidence that it works, I looked it over some more and now like this a lot. It is definitely more readable

Re: [HACKERS] SSI patch version 14

2011-02-01 Thread Kevin Grittner
Jeff Davis pg...@j-davis.com wrote: One thing that confused me a little about the code is the default case at the end. The enum is exhaustive, so the default doesn't really make sense. The compiler warning you are silencing is the uninitialized variable xid (right?) Right. Since you

Re: [HACKERS] SSI patch version 14

2011-02-01 Thread Jeff Davis
On Tue, 2011-02-01 at 11:01 -0600, Kevin Grittner wrote: My compiler doesn't. Strange. Maybe it requires -O2? Would it make sense to elog here, rather than Assert? I'm not clear on the rules for that. elog looks fine there to me, assuming we have the default case. I'm not 100% clear on the

Re: [HACKERS] SSI patch version 14

2011-02-01 Thread Kevin Grittner
Jeff Davis pg...@j-davis.com wrote: On Tue, 2011-02-01 at 11:01 -0600, Kevin Grittner wrote: My compiler doesn't. Strange. Maybe it requires -O2? That's not it; I see -O2 in my compiles. At any rate, I think the default clause is the best place to quash the warning because that leaves us

Re: [HACKERS] SSI patch version 14

2011-01-31 Thread Dan Ports
On Sun, Jan 30, 2011 at 04:01:56PM -0600, Kevin Grittner wrote: I'm wondering how this differs from what is discussed in Section 2.7 (Serialization Graph Testing) of Cahill's doctoral thesis. That discusses a technique for trying to avoid false positives by testing the full graph for cycles,

Re: [HACKERS] SSI patch version 14

2011-01-31 Thread Kevin Grittner
Jeff Davis wrote: 1. In CheckForSerializableConflictIn(), I think the comment above may be out of date. It says: 2. Also in the comment above CheckForSerializableConflictIn(), I see: 3. The comment above CheckForSerializableConflictOut() seems to trail off, as though you may have

Re: [HACKERS] SSI patch version 14

2011-01-31 Thread Jeff Davis
On Mon, 2011-01-31 at 07:26 -0600, Kevin Grittner wrote: And why are you reading the infomask directly? Do the existing visibility functions not suffice? It's possible we re-invented some code somewhere, but I'm not clear on what code from this patch might use what existing function.

Re: [HACKERS] SSI patch version 14

2011-01-31 Thread Kevin Grittner
Jeff Davis pg...@j-davis.com wrote: On Mon, 2011-01-31 at 07:26 -0600, Kevin Grittner wrote: And why are you reading the infomask directly? Do the existing visibility functions not suffice? It's possible we re-invented some code somewhere, but I'm not clear on what code from this patch

Re: [HACKERS] SSI patch version 14

2011-01-31 Thread Jeff Davis
On Mon, 2011-01-31 at 13:32 -0600, Kevin Grittner wrote: Ah, now I see what you're talking about. Take a look at where that valid flag come from -- the CheckForSerializableConflictOut are all place right after calls to HeapTupleSatisfiesVisibility. The valid value is what

Re: [HACKERS] SSI patch version 14

2011-01-31 Thread Kevin Grittner
Jeff Davis pg...@j-davis.com wrote: I don't think this function really cares about the visibility with respect to the current snapshot, right? What it cares about is whether some other particular top level transaction wrote a tuple which we *would* read except that it is not visible to us

Re: [HACKERS] SSI patch version 14

2011-01-31 Thread Kevin Grittner
Jeff Davis pg...@j-davis.com wrote: Really, I think this should be using HTSV to separate concerns better and improve readability. My first reaction was to try to find out what the function was doing that's special. If it is doing something special, and HTSV is not what you're really

Re: [HACKERS] SSI patch version 14

2011-01-31 Thread Jeff Davis
On Mon, 2011-01-31 at 13:55 -0600, Kevin Grittner wrote: Jeff Davis pg...@j-davis.com wrote: I don't think this function really cares about the visibility with respect to the current snapshot, right? What it cares about is whether some other particular top level transaction wrote a

Re: [HACKERS] SSI patch version 14

2011-01-31 Thread Kevin Grittner
Jeff Davis pg...@j-davis.com wrote: On Mon, 2011-01-31 at 13:55 -0600, Kevin Grittner wrote: What it cares about is whether some other particular top level transaction wrote a tuple which we *would* read except that it is not visible to us because that other top level transaction is

Re: [HACKERS] SSI patch version 14

2011-01-31 Thread Kevin Grittner
I wrote: We follow this by a check for the top-level xid, and return if that's early enough to have overlapped our transaction. s/early enough to have overlapped/early enough not to have overlapped/ -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make

Re: [HACKERS] SSI patch version 14

2011-01-31 Thread Jeff Davis
On Mon, 2011-01-31 at 14:38 -0600, Kevin Grittner wrote: It is at least as likely that I'm missing something. If I'm following you, we're talking about these 24 lines of code, where valid is the what was just returned from HeapTupleSatisfiesVisibility: Yes. (1) Do you see a case where

Re: [HACKERS] SSI patch version 14

2011-01-31 Thread Kevin Grittner
Jeff Davis pg...@j-davis.com wrote: Ok, great. When I read that before I thought that WAL might need to be sent for implicit RO transactions. I will read it more carefully again. In looking back over recent posts to see what I might have missed or misinterpreted, I now see your point.

Re: [HACKERS] SSI patch version 14

2011-01-31 Thread Kevin Grittner
Jeff Davis pg...@j-davis.com wrote: On Mon, 2011-01-31 at 14:38 -0600, Kevin Grittner wrote: If I want to try the switch statement from your recent post, what should I use as the OldestXmin value on the call to HTSV? I believe RecentGlobalXmin should work. And I don't think the

Re: [HACKERS] SSI patch version 14

2011-01-31 Thread Jeff Davis
On Mon, 2011-01-31 at 15:30 -0600, Kevin Grittner wrote: I'll try to set this up and see if I can get it to pass the check and dcheck make targets. Can we assume that the performance impact would be too small to matter when we know for sure that hint bits have already been set? I think

Re: [HACKERS] SSI patch version 14

2011-01-31 Thread Kevin Grittner
Jeff Davis pg...@j-davis.com wrote: On Mon, 2011-01-31 at 15:30 -0600, Kevin Grittner wrote: I'll try to set this up and see if I can get it to pass the check and dcheck make targets. Can we assume that the performance impact would be too small to matter when we know for sure that hint bits

Re: [HACKERS] SSI patch version 14

2011-01-30 Thread Kevin Grittner
Dan Ports wrote: On Thu, Jan 27, 2011 at 09:18:23AM -0800, Jeff Davis wrote: Is there a reason we can't check for a real cycle, which would let T1 succeed? I talked today with someone who experimented with doing exactly that in MySQL as part of a research project (Perfectly Serializable

Re: [HACKERS] SSI patch version 14

2011-01-30 Thread Jeff Davis
1. In CheckForSerializableConflictIn(), I think the comment above may be out of date. It says: A tuple insert is in conflict only if there is a predicate lock against the entire relation. That doesn't appear to be true if, for example, there's a predicate lock on the index page that the tuple

Re: [HACKERS] SSI patch version 14

2011-01-28 Thread Jeff Davis
On Tue, 2011-01-25 at 15:22 -0600, Kevin Grittner wrote: Jeff Davis pg...@j-davis.com wrote: I think just annotating RWConflict.in/outLink and PredTranList.available/activeList with the types of things they hold would be a help. Also, you say something about RWConflict and when the

Re: [HACKERS] SSI patch version 14

2011-01-28 Thread Dan Ports
On Thu, Jan 27, 2011 at 09:18:23AM -0800, Jeff Davis wrote: On Tue, 2011-01-25 at 05:57 -0500, Dan Ports wrote: This summary is right on. I would add one additional detail or clarification to the last point, which is that rather than checking for a cycle, we're checking for a transaction

Re: [HACKERS] SSI patch version 14

2011-01-27 Thread Kevin Grittner
I wrote: While the overall code coverage for PostgreSQL source code is: Overall coverage rate: lines..: 64.8% (130296 of 201140 lines) functions..: 72.0% (7997 of 11109 functions) By the way, I discovered that the above is lower if you just run the check target. The dcheck

Re: [HACKERS] SSI patch version 14

2011-01-27 Thread Jeff Davis
On Tue, 2011-01-25 at 05:57 -0500, Dan Ports wrote: This summary is right on. I would add one additional detail or clarification to the last point, which is that rather than checking for a cycle, we're checking for a transaction with both in and out conflicts, which every cycle must contain.

Re: [HACKERS] SSI patch version 14

2011-01-27 Thread Kevin Grittner
Jeff Davis pg...@j-davis.com wrote: To clarify, this means that it will get some false positives, right? Yes. But the example you're about to get into isn't one of them. For instance: T1: get snapshot T2: get snapshot insert R1 commit T1: read R1 write R2

Re: [HACKERS] SSI patch version 14

2011-01-27 Thread Kevin Grittner
Kevin Grittner kevin.gritt...@wicourts.gov wrote: Now, with serializable transactions, as you saw, T1 will be rolled back. I should probably have mentioned, that if all the transactions were SERIALIZABLE and the report of transactions from the batch was run as SERIALIZABLE READ ONLY

Re: [HACKERS] SSI patch version 14

2011-01-26 Thread Simon Riggs
On Mon, 2011-01-24 at 21:30 -0600, Kevin Grittner wrote: Dan and I have spent many, many hours desk-check and testing, including pounding for many hours in DBT-2 at high concurrency factors on a 16 processor box. In doing so, we found and fixed a few issues. Neither of us is aware of any bugs

Re: [HACKERS] SSI patch version 14

2011-01-26 Thread Simon Riggs
On Wed, 2011-01-26 at 11:36 +, Simon Riggs wrote: Pounding for hours on 16 CPU box sounds good. What diagnostics or instrumentation are included with the patch? How will we know whether pounding for hours is actually touching all relevant parts of code? I've done such things myself only

Re: [HACKERS] SSI patch version 14

2011-01-26 Thread Kevin Grittner
Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: On 25.01.2011 22:53, Kevin Grittner wrote: Is there somewhere you would like to see that argument documented? README-SSI . Done (borrowing some of your language).

Re: [HACKERS] SSI patch version 14

2011-01-26 Thread Kevin Grittner
Simon Riggs si...@2ndquadrant.com wrote: On Wed, 2011-01-26 at 11:36 +, Simon Riggs wrote: Pounding for hours on 16 CPU box sounds good. What diagnostics or instrumentation are included with the patch? How will we know whether pounding for hours is actually touching all relevant parts

Re: [HACKERS] SSI patch version 14

2011-01-26 Thread Kevin Grittner
Simon Riggs si...@2ndquadrant.com wrote: I looked at the patch to begin a review and immediately saw dtest. There's no docs to explain what it is, but a few comments fill me in a little more. Can we document that please? And/or explain why its an essential part of this commit? Could we keep

Re: [HACKERS] SSI patch version 14

2011-01-26 Thread Markus Wanner
Kevin, thanks for your heads-up. On 01/26/2011 06:07 PM, Kevin Grittner wrote: Simon Riggs si...@2ndquadrant.com wrote: I looked at the patch to begin a review and immediately saw dtest. There's no docs to explain what it is, but a few comments fill me in a little more. Can we document

Re: [HACKERS] SSI patch version 14

2011-01-26 Thread Simon Riggs
On Wed, 2011-01-26 at 11:07 -0600, Kevin Grittner wrote: When this runs in production, how will we know whether SSI is stuck Stuck? I'm not sure what you mean by that. Other than LW locking (which I believe is always appropriately brief, with rules for order of acquisition which

Re: [HACKERS] SSI patch version 14

2011-01-26 Thread Alvaro Herrera
Excerpts from Kevin Grittner's message of mié ene 26 14:07:18 -0300 2011: Simon Riggs si...@2ndquadrant.com wrote: Pounding for hours on 16 CPU box sounds good. What diagnostics or instrumentation are included with the patch? How will we know whether pounding for hours is actually touching

Re: [HACKERS] SSI patch version 14

2011-01-26 Thread Kevin Grittner
Alvaro Herrera alvhe...@commandprompt.com wrote: BTW did you try make coverage and friends? See http://www.postgresql.org/docs/9.0/static/regress-coverage.html and http://developer.postgresql.org/~petere/coverage/ I had missed that; thanks for pointing it out! I'm doing a coverage build

Re: [HACKERS] SSI patch version 14

2011-01-26 Thread Kevin Grittner
Kevin Grittner kevin.gritt...@wicourts.gov wrote: Alvaro Herrera alvhe...@commandprompt.com wrote: BTW did you try make coverage and friends? See http://www.postgresql.org/docs/9.0/static/regress-coverage.html and http://developer.postgresql.org/~petere/coverage/ I had missed that;

Re: [HACKERS] SSI patch version 14

2011-01-26 Thread Dan Ports
On Wed, Jan 26, 2011 at 10:01:28AM -0600, Kevin Grittner wrote: In looking at it just now, I noticed that after trying it in a couple different places what was left in the repository was not the optimal version for code coverage. I've put this back to the version which did a better job, for

Re: [HACKERS] SSI patch version 14

2011-01-26 Thread Dan Ports
On Wed, Jan 26, 2011 at 01:42:23PM -0600, Kevin Grittner wrote: Dan, do you still have access to that machine you were using for the DBT-2 runs? Could we get a coverage run with and without TEST_OLDSERXID defined? Sure, I'll give it a shot (once I figure out how to enable coverage...) Dan

  1   2   >