Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-09-14 Thread Thomas Munro
On 12 September 2014 03:56, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Thomas Munro wrote: But to reach the case you mentioned, it would need to get past that (xmax is not a valid transaction) but then the tuple would need to be locked by another session before heap_lock_tuple is called a

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-09-11 Thread Thomas Munro
On 10 September 2014 14:47, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Thomas Munro wrote: @@ -2022,7 +2030,7 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode, bool noWait, */ test = heap_lock_tuple(relation, tuple,

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-09-11 Thread Alvaro Herrera
Thomas Munro wrote: Doesn't this heap_lock_tuple() need to check for a WouldBlock result? Not sure that this is the same case that you were trying to test in heap_lock_updated_tuple; I think this requires an update chain (so that EPQFetch is invoked) and some tuple later in the chain is

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-09-10 Thread Alvaro Herrera
Thomas Munro wrote: @@ -2022,7 +2030,7 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode, bool noWait, */ test = heap_lock_tuple(relation, tuple,

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-09-10 Thread Robert Haas
On Wed, Sep 10, 2014 at 9:47 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: I attach some additional minor suggestions to your patch. I don't see any attachment. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-09-10 Thread Alvaro Herrera
Robert Haas wrote: On Wed, Sep 10, 2014 at 9:47 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: I attach some additional minor suggestions to your patch. I don't see any attachment. Uh. Here it is. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development,

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-09-06 Thread Thomas Munro
On 31 August 2014 01:36, Thomas Munro mu...@ip9.org wrote: On 28 August 2014 00:25, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Thomas Munro wrote: I haven't yet figured out how to get get into a situation where heap_lock_updated_tuple_rec waits. Well, as I think I said in the first post

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-08-30 Thread Thomas Munro
On 28 August 2014 00:25, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Thomas Munro wrote: I haven't yet figured out how to get get into a situation where heap_lock_updated_tuple_rec waits. Well, as I think I said in the first post I mentioned this, maybe there is no such situation. In any

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-08-28 Thread Thomas Munro
On 28 August 2014 00:25, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Thomas Munro wrote: Thanks, I hadn't seen this, I should have checked the archives better. I have actually already updated my patch to handle EvalPlanQualFetch with NOWAIT and SKIP LOCKED with isolation specs, see

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-08-27 Thread Alvaro Herrera
Thomas Munro wrote: On 25 August 2014 02:57, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Thomas Munro wrote: The difficulty of course will be testing all these racy cases reproducibly... Does this help? http://www.postgresql.org/message-id/51fb4305.3070...@2ndquadrant.com The

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-08-27 Thread Thomas Munro
On 27 August 2014 17:18, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Thomas Munro wrote: On 25 August 2014 02:57, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Thomas Munro wrote: The difficulty of course will be testing all these racy cases reproducibly... Does this help?

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-08-27 Thread Alvaro Herrera
Thomas Munro wrote: On 27 August 2014 17:18, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Thomas Munro wrote: Yes it does, thanks Alvaro and Craig. I think the attached spec reproduces the problem using that trick, ie shows NOWAIT blocking, presumably in EvalPlanQualFetch (though I

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-08-27 Thread Alvaro Herrera
Thomas Munro wrote: Thanks, I hadn't seen this, I should have checked the archives better. I have actually already updated my patch to handle EvalPlanQualFetch with NOWAIT and SKIP LOCKED with isolation specs, see attached. I will compare with Craig's and see if I screwed anything up... of

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-08-25 Thread Alvaro Herrera
Thomas Munro wrote: On 22 August 2014 23:02, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Forgive me if I have misunderstood but it looks like your incremental patch included a couple of unrelated changes, namely s/0/InvalidCommandId/ and a reversion of ConditionalMultiXactIdWait. Yeah,

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-08-25 Thread Heikki Linnakangas
On 07/31/2014 12:29 AM, Thomas Munro wrote: On 29 July 2014 02:35, Alvaro Herrera alvhe...@2ndquadrant.com wrote: David Rowley wrote: I've also been looking at the isolation tests and I see that you've added a series of tests for NOWAIT. I was wondering why you did that as that's really

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-08-25 Thread Thomas Munro
On 25 August 2014 02:57, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Thomas Munro wrote: The difficulty of course will be testing all these racy cases reproducibly... Does this help? http://www.postgresql.org/message-id/51fb4305.3070...@2ndquadrant.com The useful trick there is forcing a

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-08-24 Thread Thomas Munro
On 22 August 2014 23:02, Alvaro Herrera alvhe...@2ndquadrant.com wrote: heap_lock_tuple() has the following comment on top: * In the failure cases, the routine fills *hufd with the tuple's t_ctid, * t_xmax (resolving a possible MultiXact, if necessary), and t_cmax * (the last only for

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-08-24 Thread Thomas Munro
On 24 August 2014 22:04, Thomas Munro mu...@ip9.org wrote: On 22 August 2014 23:02, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Did you consider heap_lock_updated_tuple? A rationale for saying it doesn't need to pay attention to the wait policy is: if you're trying to lock-skip-locked an

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-08-24 Thread Craig Ringer
On 08/25/2014 09:44 AM, Thomas Munro wrote: On 24 August 2014 22:04, Thomas Munro mu...@ip9.org wrote: On 22 August 2014 23:02, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Did you consider heap_lock_updated_tuple? A rationale for saying it doesn't need to pay attention to the wait policy

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-08-24 Thread Alvaro Herrera
Thomas Munro wrote: While trying to produce the heap_lock_updated_tuple_rec case you describe (so far unsuccessfully), I discovered I could make SELECT ... FOR UPDATE NOWAIT block indefinitely on unpatched 9.3 in a different code path after heap_lock_tuple returns: in another session, UPDATE,

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-08-22 Thread Alvaro Herrera
One thing I just noticed is that we uselessly set an error context callback when waiting in ConditionalMultiXactIdWait, which is pretty useless (because we don't actually wait there at all) -- we don't set one in ConditionalXactLockTableWait, which makes sense, but for some reason I failed to

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-08-22 Thread Alvaro Herrera
heap_lock_tuple() has the following comment on top: * In the failure cases, the routine fills *hufd with the tuple's t_ctid, * t_xmax (resolving a possible MultiXact, if necessary), and t_cmax * (the last only for HeapTupleSelfUpdated, since we * cannot obtain cmax from a combocid generated

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-08-04 Thread Alvaro Herrera
David Rowley wrote: The only notes I can think to leave for the commiter would be around the precedence order of the lock policy, especially around a query such as: SELECT * FROM (SELECT * FROM a FOR UPDATE SKIP LOCKED) a FOR UPDATE; -- skip locked wins Of course the current behaviour is

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-08-01 Thread David Rowley
On Tue, Jul 29, 2014 at 1:35 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: David Rowley wrote: I've also been looking at the isolation tests and I see that you've added a series of tests for NOWAIT. I was wondering why you did that as that's really existing code, probably if you

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-08-01 Thread David Rowley
On Tue, Jul 29, 2014 at 9:48 AM, Thomas Munro mu...@ip9.org wrote: On 27 July 2014 23:19, Thomas Munro mu...@ip9.org wrote: On the subject of isolation tests, I think skip-locked.spec is only producing schedules that reach third of the three 'return HeapTupleWouldBlock' statements in

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-08-01 Thread Thomas Munro
On 1 August 2014 10:37, David Rowley dgrowle...@gmail.com wrote: * skip-locked-2.spec # s2 againt skips because it can't acquired a multi-xact lock againt should be again Fixed. also mixed use of multixact and multi-xact, probably would be better to stick to just 1. Changed to multixact

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-08-01 Thread David Rowley
On Sat, Aug 2, 2014 at 3:55 AM, Thomas Munro mu...@ip9.org wrote: On 1 August 2014 10:37, David Rowley dgrowle...@gmail.com wrote: Apart from this I can't see any other problems with the patch and I'd be very inclined, once the above are fixed up to mark the patch ready for commiter.

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-07-30 Thread Thomas Munro
On 29 July 2014 02:35, Alvaro Herrera alvhe...@2ndquadrant.com wrote: David Rowley wrote: I've also been looking at the isolation tests and I see that you've added a series of tests for NOWAIT. I was wondering why you did that as that's really existing code, probably if you thought the tests

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-07-28 Thread Thomas Munro
On 27 July 2014 23:19, Thomas Munro mu...@ip9.org wrote: On the subject of isolation tests, I think skip-locked.spec is only producing schedules that reach third of the three 'return HeapTupleWouldBlock' statements in heap_lock_tuple. I will follow up with some more thorough isolation tests

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-07-28 Thread Alvaro Herrera
Tom Lane wrote: It might be better if we'd declared AclMode in a single-purpose header, say utils/aclmode.h, and then #include'd that into parsenodes.h. There's certainly plenty of other single-datatype headers laying about. Do you mean src/include/datatype/aclmode.h? -- Álvaro Herrera

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-07-28 Thread Alvaro Herrera
David Rowley wrote: I've also been looking at the isolation tests and I see that you've added a series of tests for NOWAIT. I was wondering why you did that as that's really existing code, probably if you thought the tests were a bit thin around NOWAIT then maybe that should be a separate

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-07-28 Thread Tom Lane
Alvaro Herrera alvhe...@2ndquadrant.com writes: Tom Lane wrote: It might be better if we'd declared AclMode in a single-purpose header, say utils/aclmode.h, and then #include'd that into parsenodes.h. There's certainly plenty of other single-datatype headers laying about. Do you mean

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-07-27 Thread David Rowley
On Sun, Jul 27, 2014 at 4:49 AM, Thomas Munro mu...@ip9.org wrote: Here is a new version of the patch with a single enum LockWaitPolicy defined in utils/lockwaitpolicy.h. That seems much cleaner A few more comments: You seem to have lost the comment which indicates that the values of the

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-07-27 Thread Thomas Munro
On 27 July 2014 14:31, David Rowley dgrowle...@gmail.com wrote: On Sun, Jul 27, 2014 at 4:49 AM, Thomas Munro mu...@ip9.org wrote: Here is a new version of the patch with a single enum LockWaitPolicy defined in utils/lockwaitpolicy.h. That seems much cleaner A few more comments: You seem

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-07-26 Thread Thomas Munro
On 24 July 2014 00:52, Thomas Munro mu...@ip9.org wrote: On 23 July 2014 13:15, David Rowley dgrowle...@gmail.com wrote: I'm also wondering about this block of code in general: if (erm-waitPolicy == RWP_WAIT) wait_policy = LockWaitBlock; else if (erm-waitPolicy == RWP_SKIP ) wait_policy =

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-07-26 Thread David Rowley
On Sat, Jul 26, 2014 at 9:34 PM, Thomas Munro mu...@ip9.org wrote: I couldn't find an existing reasonable place to share a single wait policy enumeration between parser/planner/executor and the heap access module, and I get the feeling that it would be unacceptable to introduce one. I guess

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-07-26 Thread Tom Lane
Thomas Munro mu...@ip9.org writes: I couldn't find an existing reasonable place to share a single wait policy enumeration between parser/planner/executor and the heap access module, and I get the feeling that it would be unacceptable to introduce one. There is a precedent in the form of

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-07-26 Thread Thomas Munro
On 26 July 2014 15:43, Tom Lane t...@sss.pgh.pa.us wrote: Thomas Munro mu...@ip9.org writes: I couldn't find an existing reasonable place to share a single wait policy enumeration between parser/planner/executor and the heap access module, and I get the feeling that it would be unacceptable to

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-07-24 Thread Thomas Munro
On 24 July 2014 00:52, Thomas Munro mu...@ip9.org wrote: On 23 July 2014 13:15, David Rowley dgrowle...@gmail.com wrote: I'm also wondering about this block of code in general: if (erm-waitPolicy == RWP_WAIT) wait_policy = LockWaitBlock; else if (erm-waitPolicy == RWP_SKIP ) wait_policy =

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-07-23 Thread David Rowley
On Sun, Jun 29, 2014 at 9:01 PM, Thomas Munro mu...@ip9.org wrote: Please find attached a rebased version of my SKIP LOCKED patch (formerly SKIP LOCKED DATA), updated to support only the Oracle-like syntax. Hi Thomas, Apologies for taking this long to get to reviewing this, I'd gotten a

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-06-29 Thread Simon Riggs
On 29 June 2014 10:01, Thomas Munro mu...@ip9.org wrote: Would anyone who is interested in a SKIP LOCKED feature and attending the CHAR(14)/PGDay UK conference next week be interested in a birds-of-a-feather discussion? Sounds like a plan. I'll check my schedule. -- Simon Riggs

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-05-23 Thread Robert Haas
On Sat, May 17, 2014 at 1:02 AM, Craig Ringer cr...@2ndquadrant.com wrote: We have a long tradition of trying to allow noise keywords where it's harmless. So the clause should probably be SKIP LOCKED [DATA] in much the same way we have BEGIN [ WORK | TRANSACTION ] ... There

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-05-23 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Sat, May 17, 2014 at 1:02 AM, Craig Ringer cr...@2ndquadrant.com wrote: We have a long tradition of trying to allow noise keywords where it's harmless. So the clause should probably be SKIP LOCKED [DATA] in much the same way we have BEGIN

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-05-23 Thread Thomas Munro
On 23 May 2014 15:40, Tom Lane t...@sss.pgh.pa.us wrote: A different concern is that this patch adds not one but two new unreserved keywords, ie SKIP and LOCKED. That bloats our parser tables, which are too darn large already, and it has a nonzero compatibility cost (since we only allow

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-05-23 Thread Simon Riggs
On 23 May 2014 10:40, Tom Lane t...@sss.pgh.pa.us wrote: If we're pulling syntax out of the air it'd be nice if we could avoid adding new keywords to the grammar. Oracle, SQLServer and DB2 have this capability. MySQL does not. SQLServer implements that using the table hint of READPAST. Since

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-05-23 Thread Pavel Stehule
2014-05-23 21:24 GMT+02:00 Simon Riggs si...@2ndquadrant.com: On 23 May 2014 10:40, Tom Lane t...@sss.pgh.pa.us wrote: If we're pulling syntax out of the air it'd be nice if we could avoid adding new keywords to the grammar. Oracle, SQLServer and DB2 have this capability. MySQL does not.

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-05-23 Thread Robert Haas
On Fri, May 23, 2014 at 3:24 PM, Simon Riggs si...@2ndquadrant.com wrote: PostgreSQL already chose to follow the Oracle syntax when we implemented NOWAIT. So my proposal is that we follow the Oracle syntax again and use the words SKIP LOCKED. I don't see any advantage in inventing new syntax

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-05-19 Thread Thomas Munro
Hello As a simple example for people wondering what the point of this feature is, I created a table work (id, data, status) and then create 10,000 items with status 'NEW' and then started a number of worker threads that did the following pair of transactions, with and without SKIP LOCKED DATA on

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-05-16 Thread Craig Ringer
I'm rebasing another implementation of this against current HEAD at the moment. It was well tested but has bitrotted a bit, in particular it needs merging with the multixact changes (eep). That should provide a useful basis for comparison and a chance to share ideas. I'll follow up with the

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-05-16 Thread Craig Ringer
On 05/16/2014 04:46 PM, Craig Ringer wrote: I'll follow up with the patch and a git tree when it's ready, hopefully tonight. Here's a rebased version of Simon's original patch that runs on current master. I still need to merge the isolation tests for it merged and sorted out, and after

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-05-16 Thread Thomas Munro
On 16 May 2014 13:21, Craig Ringer cr...@2ndquadrant.com wrote: On 05/16/2014 04:46 PM, Craig Ringer wrote: I'll follow up with the patch and a git tree when it's ready, hopefully tonight. Here's a rebased version of Simon's original patch that runs on current master. I still need to

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-05-16 Thread Craig Ringer
On 05/17/2014 05:24 AM, Thomas Munro wrote: I noticed that in applyLockingClause, Simon has rc-waitMode |= waitMode. Is that right? The values are 0, 1, and 2, but if you had both NOWAIT and SKIP LOCKED somewhere in your query you could up with rc-waitMode == 3 unless I'm mistaken. I do not

Re: [HACKERS] SKIP LOCKED DATA (work in progress)

2014-05-13 Thread Craig Ringer
On 05/14/2014 07:06 AM, Thomas Munro wrote: Hi A couple of years ago I posted an outline of a plan [1] and an initial patch [2] for implementing SKIP LOCKED DATA. I have recently come back to this idea, rebased the patch and added a simple isolation test -- please see attached. Simon did