Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 few lines
below. That's a race scenario that I don't believe we can create
using advisory lock tricks in an isolation test.
Hm, are you able to reproduce it using GDB?
Craig Ringer was saying elsewhere that there are other cases that are
impossible to test reliably and was proposing addings hooks or
something to block backends at convenient times. Not an easy problem ...
+1, I think that is a great idea.
FWIW here's some throwaway code that I used to do that:
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 79667f1..fbb3b55 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -54,6 +54,7 @@
#include storage/lmgr.h
#include tcop/utility.h
#include utils/acl.h
+#include utils/builtins.h
#include utils/lsyscache.h
#include utils/memutils.h
#include utils/snapmgr.h
@@ -2029,6 +2030,20 @@ EvalPlanQualFetch(EState *estate, Relation
relation, int lockmode,
}
/*
+* Begin wait point debugging hack...
+* TODO: Only in a special build mode...
+* We tell anyone waiting that we have reached
wait point #42.
+* We wait for permission to proceed from wait
point #43.
+*/
+ elog(WARNING, XXX reached point 42, waiting
at point 43);
+ DirectFunctionCall1(pg_advisory_unlock_int8,
Int64GetDatum(42));
+ DirectFunctionCall1(pg_advisory_lock_int8,
Int64GetDatum(43));
+ elog(WARNING, XXX continuing after point 43);
+ /*
+* End wait point debugging hack.
+*/
+
+ /*
* This is a live tuple, so now try to lock it.
*/
test = heap_lock_tuple(relation, tuple,
Using the attached isolation spec, that race case is reached. Yeah,
it's crude and confusing having those three advisory locks (one to
allow an update chain to be created after s1 takes a snapshot, and the
other two so that s2 can block s1 at the right point to produce that
race case), but I found this less messy than trying to reproduce
complicated concurrency scenarios with GDB.
IMHO it would be great if there were a tidy and supported way to do
this kind of thing, perhaps with a formal notion of named wait points
which are only compiled in in special test builds, and an optional set
of extra isolation specs that use them.
I attach some additional minor suggestions to your patch. Please feel
free to reword comments differently if you think my wording isn't an
improvements (or I've maked an english mistakes).
Thanks, these are incorporated in the new version (also rebased).
Great, thanks; I'll look at it again soon to commit, as I think we're
done now.
Thanks!
Thomas Munro
# Test SKIP LOCKED with an updated tuple chain, race case with wait at
# control point #42
setup
{
CREATE TABLE foo (
id int PRIMARY KEY,
data text NOT NULL
);
INSERT INTO foo VALUES (1, 'x'), (2, 'x');
}
teardown
{
DROP TABLE foo;
}
session s1
setup {
-- we hold a lock that s2c can wait for
SELECT pg_advisory_lock(42);
BEGIN;
}
step s1a { SELECT * FROM foo WHERE pg_advisory_lock(0) IS NOT NULL ORDER BY id LIMIT 1 FOR UPDATE SKIP LOCKED; }
step s1b { COMMIT; }
session s2
step s2a {
-- first, block s1a from running after it has taken its snapshot
SELECT pg_advisory_lock(0);
}
step s2b {
-- generate up update chain and commit
UPDATE foo SET data = data WHERE id = 1;
}
step s2c {
-- unblock s1a so that it starts running but make it wait at control point 43
SELECT pg_advisory_lock(43);
SELECT pg_advisory_unlock(0);
}
step s2d {
-- wait for s1a to reach wait point 42
SELECT pg_advisory_lock(42);
-- now lock the tuple and hold the lock
BEGIN;
UPDATE foo SET data = data WHERE id = 1;
-- and finally allow s1a to continue
SELECT pg_advisory_unlock(43);
}
step s2e { COMMIT; }
permutation s2a s1a s2b s2c s2d s1b s2e
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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,
estate-es_output_cid,
-lockmode,
noWait,
+lockmode,
wait_policy,
false,
buffer, hufd);
/* We now have two pins on the buffer, get rid of one
*/
ReleaseBuffer(buffer);
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 locked by a
third transaction.
You're right, that case was clearly lacking. The new patch, attached,
releases the buffer and returns NULL in that case. But I have no idea
how to reach it in an isolation test! skip-locked-4.spec gets pretty
close, it creates the right kind of update chain to get into
EvalPlanQualFetch and then enter the conditional block beginning:
if (TransactionIdIsValid(SnapshotDirty.xmax))
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 few lines
below. That's a race scenario that I don't believe we can create
using advisory lock tricks in an isolation test.
I attach some additional minor suggestions to your patch. Please feel
free to reword comments differently if you think my wording isn't an
improvements (or I've maked an english mistakes).
Thanks, these are incorporated in the new version (also rebased).
Best regards,
Thomas Munro
diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index 940d1aa..9d5da19 100644
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -45,7 +45,7 @@ SELECT [ ALL | DISTINCT [ ON ( replaceable class=parameterexpression/replac
[ LIMIT { replaceable class=parametercount/replaceable | ALL } ]
[ OFFSET replaceable class=parameterstart/replaceable [ ROW | ROWS ] ]
[ FETCH { FIRST | NEXT } [ replaceable class=parametercount/replaceable ] { ROW | ROWS } ONLY ]
-[ FOR { UPDATE | NO KEY UPDATE | SHARE | KEY SHARE } [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT ] [...] ]
+[ FOR { UPDATE | NO KEY UPDATE | SHARE | KEY SHARE } [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT | SKIP LOCKED ] [...] ]
phrasewhere replaceable class=parameterfrom_item/replaceable can be one of:/phrase
@@ -1284,7 +1284,7 @@ FETCH { FIRST | NEXT } [ replaceable class=parametercount/replaceable ] {
The locking clause has the general form
synopsis
-FOR replaceablelock_strength/ [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT ]
+FOR replaceablelock_strength/ [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT | SKIP LOCKED ]
/synopsis
where replaceablelock_strength/ can be one of
@@ -1360,11 +1360,17 @@ KEY SHARE
para
To prevent the operation from waiting for other transactions to commit,
-use the literalNOWAIT/ option. With literalNOWAIT/, the statement
-reports an error, rather than waiting, if a selected row
-cannot be locked immediately. Note that literalNOWAIT/ applies only
-to the row-level lock(s) mdash; the required literalROW SHARE/literal
-table-level lock is still taken in the ordinary way (see
+use either the literalNOWAIT/ or literalSKIP LOCKED/literal
+option. With literalNOWAIT/, the statement reports an error, rather
+than waiting, if a selected row cannot be locked immediately.
+With literalSKIP LOCKED/literal, any selected rows that cannot be
+immediately locked are skipped. Skipping locked rows provides an
+inconsistent view of the data, so this is not suitable for general purpose
+work, but can be used to avoid lock contention with multiple consumers
+accessing a queue-like table. Note that literalNOWAIT/
+and literalSKIP LOCKED/literal apply only to the row-level lock(s)
+mdash; the required literalROW SHARE/literal table-level lock is
+still taken in the ordinary way (see
xref linkend=mvcc). You can use
xref linkend=sql-lock
with the literalNOWAIT/ option first,
@@ -1387,14 +1393,14 @@ KEY SHARE
/para
para
-Multiple locking
-clauses can be written if it is necessary to specify different locking
-behavior for different tables. If the same table is mentioned (or
-
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 locked by a third transaction. You're right, that case was clearly lacking. The new patch, attached, releases the buffer and returns NULL in that case. But I have no idea how to reach it in an isolation test! skip-locked-4.spec gets pretty close, it creates the right kind of update chain to get into EvalPlanQualFetch and then enter the conditional block beginning: if (TransactionIdIsValid(SnapshotDirty.xmax)) 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 few lines below. That's a race scenario that I don't believe we can create using advisory lock tricks in an isolation test. Hm, are you able to reproduce it using GDB? Craig Ringer was saying elsewhere that there are other cases that are impossible to test reliably and was proposing addings hooks or something to block backends at convenient times. Not an easy problem ... I attach some additional minor suggestions to your patch. Please feel free to reword comments differently if you think my wording isn't an improvements (or I've maked an english mistakes). Thanks, these are incorporated in the new version (also rebased). Great, thanks; I'll look at it again soon to commit, as I think we're done now. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
Thomas Munro wrote: @@ -2022,7 +2030,7 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode, bool noWait, */ test = heap_lock_tuple(relation, tuple, estate-es_output_cid, -lockmode, noWait, +lockmode, wait_policy, false, buffer, hufd); /* We now have two pins on the buffer, get rid of one */ ReleaseBuffer(buffer); 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 locked by a third transaction. I attach some additional minor suggestions to your patch. Please feel free to reword comments differently if you think my wording isn't an improvements (or I've maked an english mistakes). -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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, 24x7 Support, Training Services
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 51d1889..2b336b0 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -4090,7 +4090,7 @@ get_mxact_status_for_lock(LockTupleMode mode, bool is_update)
* cid: current command ID (used for visibility test, and stored into
* tuple's cmax if lock is successful)
* mode: indicates if shared or exclusive tuple lock is desired
- * wait_policy: whether to block, ereport or skip if lock not available
+ * wait_policy: what to do if tuple lock is not available
* follow_updates: if true, follow the update chain to also lock descendant
* tuples.
*
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index e4065c2..a718c0b 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1928,6 +1928,9 @@ EvalPlanQual(EState *estate, EPQState *epqstate,
*
* Returns a palloc'd copy of the newest tuple version, or NULL if we find
* that there is no newest version (ie, the row was deleted not updated).
+ * We also return NULL if the tuple is locked and the wait policy is to skip
+ * such tuples.
+ *
* If successful, we have locked the newest tuple version, so caller does not
* need to worry about it changing anymore.
*
@@ -1994,7 +1997,7 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
break;
case LockWaitSkip:
if (!ConditionalXactLockTableWait(SnapshotDirty.xmax))
- return NULL; /* skipping instead of waiting */
+ return NULL; /* skip instead of waiting */
break;
case LockWaitError:
if (!ConditionalXactLockTableWait(SnapshotDirty.xmax))
@@ -2076,6 +2079,10 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
/* tuple was deleted, so give up */
return NULL;
+case HeapTupleWouldBlock:
+ elog(WARNING, this is an odd case);
+ return NULL;
+
default:
ReleaseBuffer(buffer);
elog(ERROR, unrecognized heap_lock_tuple status: %u,
diff --git a/src/backend/executor/nodeLockRows.c b/src/backend/executor/nodeLockRows.c
index f9feff4..990240b 100644
--- a/src/backend/executor/nodeLockRows.c
+++ b/src/backend/executor/nodeLockRows.c
@@ -179,7 +179,10 @@ lnext:
if (copyTuple == NULL)
{
- /* Tuple was deleted or skipped (in SKIP LOCKED), so don't return it */
+ /*
+ * Tuple was deleted; or it's locked and we're under SKIP
+ * LOCKED policy, so don't return it
+ */
goto lnext;
}
/* remember the actually locked tuple's TID */
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 0f6e9b0..1f66928 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -2515,11 +2515,12 @@ applyLockingClause(Query *qry, Index rtindex,
* a shared and exclusive lock at the same time; it'll end up being
* exclusive anyway.)
*
- * We also consider that NOWAIT wins if it is specified multiple ways,
- * otherwise SKIP LOCKED wins. This is a bit more debatable but
- * raising an error doesn't seem helpful. (Consider for instance
- * SELECT FOR UPDATE NOWAIT from a view that internally contains a
- * plain FOR UPDATE spec.)
+ * Similarly, if the same RTE is specified with more than one lock wait
+ * policy, consider that NOWAIT wins over SKIP LOCKED, which in turn
+ * wins over waiting for the lock (the default). This is a bit more
+ * debatable but raising an error doesn't seem helpful. (Consider for
+ * instance SELECT FOR UPDATE NOWAIT from a view that internally
+ * contains a plain FOR UPDATE spec.)
*
* And of course pushedDown becomes false if any clause is explicit.
*/
diff --git a/src/include/utils/lockwaitpolicy.h b/src/include/utils/lockwaitpolicy.h
index f8ad07e..7f9a693 100644
--- a/src/include/utils/lockwaitpolicy.h
+++ b/src/include/utils/lockwaitpolicy.h
@@ -1,34 +1,31 @@
/*-
- *
* lockwaitpolicy.h
- * Header file for the enum LockWaitPolicy enum, which is needed in
- * several modules (parser, planner, executor, heap manager).
+ * Header file for LockWaitPolicy enum.
*
* Copyright (c) 2014, PostgreSQL Global Development Group
*
* src/include/utils/lockwaitpolicy.h
- *
*-
*/
#ifndef LOCKWAITPOLICY_H
#define LOCKWAITPOLICY_H
/*
- * Policy for what to do when a row lock cannot be obtained immediately.
- *
- * The enum values defined here control how the parser treats multiple FOR
- * UPDATE/SHARE
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 I mentioned this, maybe there is no such situation. In any case, like the EvalPlanQualFetch issue, we can fix it later if we find it. I finally came up with a NOWAIT spec that reaches heap_lock_updated_rec and then blocks. I can't explain why exactly... but please see attached. The fix seems fairly straightforward. Do you think I should submit an independent patch to fix this case (well there are really two cases, since there is a separate multixact path) for the existing NOWAIT support and then tackle the SKIP LOCKED equivalent separately? Oops, that isolation wasn't right, please disregard. Unless you think this obscure case needs to be addressed before the SKIP LOCKED patch can be committed, I will investigate it separately and maybe submit something to a future commitfest. Best regards, Thomas Munro -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 case, like the EvalPlanQualFetch issue, we
can fix it later if we find it.
I finally came up with a NOWAIT spec that reaches
heap_lock_updated_rec and then blocks. I can't explain why exactly...
but please see attached. The fix seems fairly straightforward. Do
you think I should submit an independent patch to fix this case (well
there are really two cases, since there is a separate multixact path)
for the existing NOWAIT support and then tackle the SKIP LOCKED
equivalent separately?
Best regards,
Thomas Munro
# Test NOWAIT with an updated tuple chain (heap_lock_updated_tuple case)
setup
{
CREATE TABLE foo (
id int PRIMARY KEY,
data text NOT NULL
);
INSERT INTO foo VALUES (1, 'x');
}
teardown
{
DROP TABLE foo;
}
session s1
setup { BEGIN; }
step s1a { SELECT * FROM foo WHERE pg_advisory_lock(0) IS NOT NULL FOR KEY SHARE NOWAIT; }
step s1b { COMMIT; }
session s2
setup { BEGIN; }
step s2a { SELECT pg_advisory_lock(0); }
step s2b { UPDATE foo SET id = id; UPDATE foo SET id = id + 10; }
step s2c { SELECT pg_advisory_unlock(0); }
step s2d { COMMIT; }
permutation s2a s1a s2b s2c s1b s2d
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 attached. I
will compare with Craig's and see if I screwed anything up... of
course I am happy to merge and submit a new patch on top of Craig's if
it's going to be committed.
Thanks, please rebase.
Thanks -- new patch attached.
Best regards,
Thomas Munro
diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index 231dc6a..0469705 100644
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -45,7 +45,7 @@ SELECT [ ALL | DISTINCT [ ON ( replaceable class=parameterexpression/replac
[ LIMIT { replaceable class=parametercount/replaceable | ALL } ]
[ OFFSET replaceable class=parameterstart/replaceable [ ROW | ROWS ] ]
[ FETCH { FIRST | NEXT } [ replaceable class=parametercount/replaceable ] { ROW | ROWS } ONLY ]
-[ FOR { UPDATE | NO KEY UPDATE | SHARE | KEY SHARE } [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT ] [...] ]
+[ FOR { UPDATE | NO KEY UPDATE | SHARE | KEY SHARE } [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT | SKIP LOCKED ] [...] ]
phrasewhere replaceable class=parameterfrom_item/replaceable can be one of:/phrase
@@ -1283,7 +1283,7 @@ FETCH { FIRST | NEXT } [ replaceable class=parametercount/replaceable ] {
The locking clause has the general form
synopsis
-FOR replaceablelock_strength/ [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT ]
+FOR replaceablelock_strength/ [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT | SKIP LOCKED ]
/synopsis
where replaceablelock_strength/ can be one of
@@ -1359,11 +1359,17 @@ KEY SHARE
para
To prevent the operation from waiting for other transactions to commit,
-use the literalNOWAIT/ option. With literalNOWAIT/, the statement
-reports an error, rather than waiting, if a selected row
-cannot be locked immediately. Note that literalNOWAIT/ applies only
-to the row-level lock(s) mdash; the required literalROW SHARE/literal
-table-level lock is still taken in the ordinary way (see
+use either the literalNOWAIT/ or literalSKIP LOCKED/literal
+option. With literalNOWAIT/, the statement reports an error, rather
+than waiting, if a selected row cannot be locked immediately.
+With literalSKIP LOCKED/literal, any selected rows that cannot be
+immediately locked are skipped. Skipping locked rows provides an
+inconsistent view of the data, so this is not suitable for general purpose
+work, but can be used to avoid lock contention with multiple consumers
+accessing a queue-like table. Note that literalNOWAIT/
+and literalSKIP LOCKED/literal apply only to the row-level lock(s)
+mdash; the required literalROW SHARE/literal table-level lock is
+still taken in the ordinary way (see
xref linkend=mvcc). You can use
xref linkend=sql-lock
with the literalNOWAIT/ option first,
@@ -1386,14 +1392,14 @@ KEY SHARE
/para
para
-Multiple locking
-clauses can be written if it is necessary to specify different locking
-behavior for different tables. If the same table is mentioned (or
-implicitly affected) by more than one locking clause,
-then it is processed as if it was only specified by the strongest one.
-Similarly, a table is processed
-as literalNOWAIT/ if that is specified in any of the clauses
-affecting it.
+Multiple locking clauses can be written if it is necessary to specify
+different locking behavior for different tables. If the same table is
+mentioned (or implicitly affected) by more than one locking clause, then
+it is processed as if it was only specified by the strongest one.
+Similarly, a table is processed as literalNOWAIT/ if that is specified
+in any of the clauses affecting it. Otherwise, it is processed
+as literalSKIP LOCKED/literal if that is specified in any of the
+clauses affecting it.
/para
para
@@ -1930,9 +1936,9 @@ SELECT distributors.* WHERE distributors.name = 'Westward';
productnamePostgreSQL/productname allows it in any commandSELECT/
query as well as in sub-commandSELECT/s, but this is an extension.
The literalFOR NO KEY UPDATE/, literalFOR SHARE/ and
-literalFOR KEY SHARE/ variants,
-as well as the literalNOWAIT/ option,
-do not appear in the standard.
+literalFOR KEY SHARE/ variants, as well as the literalNOWAIT/
+and literalSKIP LOCKED/literal options, do not appear in the
+standard.
/para
/refsect2
diff --git a/doc/src/sgml/sql.sgml b/doc/src/sgml/sql.sgml
index ba92607..57396d7 100644
--- a/doc/src/sgml/sql.sgml
+++ b/doc/src/sgml/sql.sgml
@@ -863,7
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 useful trick there is forcing a query to get its snapshot and then go to sleep before actually doing anything, by way of an advisory lock. 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 haven't stepped through it with a debugger yet). I'm afraid I'm out of Postgres hacking cycles for a few days, but next weekend I should have a new patch that fixes this by teaching EvalPlanQualFetch about wait policies, with isolation tests for NOWAIT and SKIP LOCKED. Hmm, http://www.postgresql.org/message-id/51fb6703.9090...@2ndquadrant.com -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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?
http://www.postgresql.org/message-id/51fb4305.3070...@2ndquadrant.com
The useful trick there is forcing a query to get its snapshot and then
go to sleep before actually doing anything, by way of an advisory lock.
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 haven't stepped through it
with a debugger yet). I'm afraid I'm out of Postgres hacking cycles
for a few days, but next weekend I should have a new patch that fixes
this by teaching EvalPlanQualFetch about wait policies, with isolation
tests for NOWAIT and SKIP LOCKED.
Hmm, http://www.postgresql.org/message-id/51fb6703.9090...@2ndquadrant.com
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
course I am happy to merge and submit a new patch on top of Craig's if
it's going to be committed.
I haven't yet figured out how to get get into a situation where
heap_lock_updated_tuple_rec waits.
Best regards,
Thomas Munro
diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index 231dc6a..0469705 100644
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -45,7 +45,7 @@ SELECT [ ALL | DISTINCT [ ON ( replaceable class=parameterexpression/replac
[ LIMIT { replaceable class=parametercount/replaceable | ALL } ]
[ OFFSET replaceable class=parameterstart/replaceable [ ROW | ROWS ] ]
[ FETCH { FIRST | NEXT } [ replaceable class=parametercount/replaceable ] { ROW | ROWS } ONLY ]
-[ FOR { UPDATE | NO KEY UPDATE | SHARE | KEY SHARE } [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT ] [...] ]
+[ FOR { UPDATE | NO KEY UPDATE | SHARE | KEY SHARE } [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT | SKIP LOCKED ] [...] ]
phrasewhere replaceable class=parameterfrom_item/replaceable can be one of:/phrase
@@ -1283,7 +1283,7 @@ FETCH { FIRST | NEXT } [ replaceable class=parametercount/replaceable ] {
The locking clause has the general form
synopsis
-FOR replaceablelock_strength/ [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT ]
+FOR replaceablelock_strength/ [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT | SKIP LOCKED ]
/synopsis
where replaceablelock_strength/ can be one of
@@ -1359,11 +1359,17 @@ KEY SHARE
para
To prevent the operation from waiting for other transactions to commit,
-use the literalNOWAIT/ option. With literalNOWAIT/, the statement
-reports an error, rather than waiting, if a selected row
-cannot be locked immediately. Note that literalNOWAIT/ applies only
-to the row-level lock(s) mdash; the required literalROW SHARE/literal
-table-level lock is still taken in the ordinary way (see
+use either the literalNOWAIT/ or literalSKIP LOCKED/literal
+option. With literalNOWAIT/, the statement reports an error, rather
+than waiting, if a selected row cannot be locked immediately.
+With literalSKIP LOCKED/literal, any selected rows that cannot be
+immediately locked are skipped. Skipping locked rows provides an
+inconsistent view of the data, so this is not suitable for general purpose
+work, but can be used to avoid lock contention with multiple consumers
+accessing a queue-like table. Note that literalNOWAIT/
+and literalSKIP LOCKED/literal apply only to the row-level lock(s)
+mdash; the required literalROW SHARE/literal table-level lock is
+still taken in the ordinary way (see
xref linkend=mvcc). You can use
xref linkend=sql-lock
with the literalNOWAIT/ option first,
@@ -1386,14 +1392,14 @@ KEY SHARE
/para
para
-Multiple locking
-clauses can be written if it is necessary to specify different locking
-behavior for different tables. If the same table is mentioned (or
-implicitly affected) by more than one locking clause,
-then it is processed as if it was only specified by the strongest one.
-Similarly, a table is processed
-as literalNOWAIT/ if that is specified in any of the clauses
-affecting it.
+Multiple locking clauses can be written if it is necessary to specify
+different locking behavior for different tables. If the same table is
+mentioned (or implicitly affected) by more than one locking clause, then
+it is processed as if it was only specified by the strongest one.
+
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 haven't stepped through it with a debugger yet). I'm afraid I'm out of Postgres hacking cycles for a few days, but next weekend I should have a new patch that fixes this by teaching EvalPlanQualFetch about wait policies, with isolation tests for NOWAIT and SKIP LOCKED. Hmm, http://www.postgresql.org/message-id/51fb6703.9090...@2ndquadrant.com 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 course I am happy to merge and submit a new patch on top of Craig's if it's going to be committed. I tried Craig's patch with your test case and found that it stalls in XactLockTableWait inside EPQFetch because it doesn't throw an error in the noWait case before waiting. I think I will fix that and push, including both test cases. Then we can see about rebasing your patch. I am wondering about backpatching Craig's fix. It looks to me like it should be backpatched as far back as NOWAIT exists, but that was in 8.1 and we haven't ever gotten a complaint until Craig's report AFAIK, which I understand wasn't coming from a user finding a problem but rather some new development. So I hesitate. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 course I am happy to merge and submit a new patch on top of Craig's if it's going to be committed. Thanks, please rebase. 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 case, like the EvalPlanQualFetch issue, we can fix it later if we find it. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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, sorry about those, will push separately. Undoing those gave me skip-locked-v12-b.patch, attached for reference, and I've included those changes in a new full patch skip-locked-v13.patch (also rebased). +1 for the change from if-then-else to switch statements. I was less sure about the 'goto failed' change, but I couldn't measure any change in concurrent throughput in my simple benchmark, so I guess that extra buffer lock/unlock doesn't matter and I can see why you prefer that control flow. I was also thinking in reducing the lock level acquired to shared rather than exclusive in all the paths that goto failed. Since the lock is only used to read a couple of fields from the tuple, shared is enough and should give slightly better concurrency. Per buffer access rules in src/backend/storage/buffer/README: : 1. To scan a page for tuples, one must hold a pin and either shared or : exclusive content lock. To examine the commit status (XIDs and status bits) : of a tuple in a shared buffer, one must likewise hold a pin and either shared : or exclusive lock. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 existing code, probably if you thought the tests were a bit thin around NOWAIT then maybe that should be a separate patch? The isolation tester is new so we don't have nearly enough tests for it. Adding more meaningful tests is good even if they're unrelated to the patch at hand. Here are my isolation tests for NOWAIT as a separate patch, independent of SKIP LOCKED. They cover the tuple lock, regular row lock and multixact row lock cases. Thanks, committed. I guess this might be called white box testing, since it usese knowledge of how to construct schedules that hit the three interesting code paths that trigger the error, even though you can't see from the output why the error was raised in each case without extra instrumentation (though it did cross my mind that it could be interesting at the very least for testing if the error message were different in each case). Yeah, seems reasonable. This kind of tests might become obsolete in the future if the internals change a lot, so that e.g. we don't use multixids anymore. But even if that happens, there's little harm in keeping the tests, and meanwhile it's good to have coverage of these cases. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 query to get its snapshot and then
go to sleep before actually doing anything, by way of an advisory lock.
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 haven't stepped through it
with a debugger yet). I'm afraid I'm out of Postgres hacking cycles
for a few days, but next weekend I should have a new patch that fixes
this by teaching EvalPlanQualFetch about wait policies, with isolation
tests for NOWAIT and SKIP LOCKED.
Best regards,
Thomas Munro
# Test NOWAIT with an updated tuple chain.
setup
{
CREATE TABLE foo (
id int PRIMARY KEY,
data text NOT NULL
);
INSERT INTO foo VALUES (1, 'x');
}
teardown
{
DROP TABLE foo;
}
session s1
setup { BEGIN; }
step s1a { SELECT * FROM foo WHERE pg_advisory_lock(0) IS NOT NULL FOR UPDATE NOWAIT; }
step s1b { COMMIT; }
session s2
step s2a { SELECT pg_advisory_lock(0); }
step s2b { UPDATE foo SET data = data; }
step s2c { BEGIN; }
step s2d { UPDATE foo SET data = data; }
step s2e { SELECT pg_advisory_unlock(0); }
step s2f { COMMIT; }
permutation s2a s1a s2b s2c s2d s2e s1b s2f
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 HeapTupleSelfUpdated, since we
* cannot obtain cmax from a combocid generated by another transaction).
* See comments for struct HeapUpdateFailureData for additional info.
With the patch as submitted, this struct is not filled in the
HeapTupleWouldBlock case. I'm not sure this is okay, though I admit the
only caller that passes LockWaitSkip doesn't care, so maybe we could
just ignore the issue (after properly modifying the comment). It seems
easy to add a LockBuffer() and goto failed rather than a return; that
would make that failure case conform to HeapTupleUpdated and
HeapTupleSelfUpdated. OTOH it might be pointless to worry about what
would be essentially dead code currently ...
Thanks Alvaro.
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.
Undoing those gave me skip-locked-v12-b.patch, attached for reference,
and I've included those changes in a new full patch
skip-locked-v13.patch (also rebased).
+1 for the change from if-then-else to switch statements.
I was less sure about the 'goto failed' change, but I couldn't measure
any change in concurrent throughput in my simple benchmark, so I guess
that extra buffer lock/unlock doesn't matter and I can see why you
prefer that control flow.
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 updated tuple, then you either skip it because its
updater is running, or you return it because it's no longer running; and
if you return it, it is not possible for the updater to be locking the
updated version. However, what if there's a third transaction that
locked the updated version? It might be difficult to hit this case or
construct an isolationtester spec file though, since there's a narrow
window you need to race to.
Hmm. I will look into this, thanks.
Best regards,
Thomas Munro
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 31073bc..464a73c 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -4222,22 +4222,27 @@ l3:
*/
if (!have_tuple_lock)
{
- if (wait_policy == LockWaitBlock)
+ switch (wait_policy)
{
-LockTupleTuplock(relation, tid, mode);
- }
- else if (wait_policy == LockWaitError)
- {
-if (!ConditionalLockTupleTuplock(relation, tid, mode))
- ereport(ERROR,
- (errcode(ERRCODE_LOCK_NOT_AVAILABLE),
- errmsg(could not obtain lock on row in relation \%s\,
- RelationGetRelationName(relation;
- }
- else /* wait_policy == LockWaitSkip */
- {
-if (!ConditionalLockTupleTuplock(relation, tid, mode))
- return HeapTupleWouldBlock;
+case LockWaitBlock:
+ LockTupleTuplock(relation, tid, mode);
+ break;
+case LockWaitSkip:
+ if (!ConditionalLockTupleTuplock(relation, tid, mode))
+ {
+ result = HeapTupleWouldBlock;
+ /* recovery code expects to have buffer lock held */
+ LockBuffer(*buffer, BUFFER_LOCK_EXCLUSIVE);
+ goto failed;
+ }
+ break;
+case LockWaitError:
+ if (!ConditionalLockTupleTuplock(relation, tid, mode))
+ ereport(ERROR,
+(errcode(ERRCODE_LOCK_NOT_AVAILABLE),
+ errmsg(could not obtain lock on row in relation \%s\,
+ RelationGetRelationName(relation;
+ break;
}
have_tuple_lock = true;
}
@@ -4441,34 +4446,36 @@ l3:
if (status = MultiXactStatusNoKeyUpdate)
elog(ERROR, invalid lock mode in heap_lock_tuple);
-/* wait for multixact to end */
-if (wait_policy == LockWaitBlock)
+/* wait for multixact to end, or die trying */
+switch (wait_policy)
{
- MultiXactIdWait((MultiXactId) xwait, status, infomask,
- relation, tuple-t_data-t_ctid,
- XLTW_Lock, NULL);
-}
-else if (wait_policy == LockWaitError)
-{
- if (!ConditionalMultiXactIdWait((MultiXactId) xwait,
- status, infomask, relation,
- tuple-t_data-t_ctid,
- XLTW_Lock, NULL))
- ereport(ERROR,
-(errcode(ERRCODE_LOCK_NOT_AVAILABLE),
- errmsg(could not obtain lock on row in relation \%s\,
- RelationGetRelationName(relation;
-}
-else /* wait_policy == LockWaitSkip */
-{
- if (!ConditionalMultiXactIdWait((MultiXactId) xwait,
- status, infomask, relation,
- tuple-t_data-t_ctid,
- XLTW_Lock, NULL))
- {
- UnlockTupleTuplock(relation, tid, mode);
- return HeapTupleWouldBlock;
- }
+ case
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 updated tuple, then you either skip it because its updater is running, or you return it because it's no longer running; and if you return it, it is not possible for the updater to be locking the updated version. However, what if there's a third transaction that locked the updated version? It might be difficult to hit this case or construct an isolationtester spec file though, since there's a narrow window you need to race to. Hmm. I will look into this, thanks. 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, COMMIT, then UPDATE, all after the first session has taken its snapshot but before it tries to lock a given row. The code in EvalPlanQualFetch (reached from the HeapTupleUpdated case in ExecLockRow) finishes up waiting for the uncommitted transaction. I think I see how to teach EvalPlanQualFetch how to handle wait policies: for NOWAIT it should ereport (fixing a pre-existing bug (?)), and I guess it should handle SKIP LOCKED by returning NULL, similarly to the way it handles deleted rows, and of course in all cases passing the wait policy forward to heap_lock_tuple, which it eventually calls. Still looking at heap_lock_updated_tuple. The difficulty of course will be testing all these racy cases reproducibly... Best regards, Thomas Munro -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 is: if you're trying to lock-skip-locked an updated tuple, then you either skip it because its updater is running, or you return it because it's no longer running; and if you return it, it is not possible for the updater to be locking the updated version. However, what if there's a third transaction that locked the updated version? It might be difficult to hit this case or construct an isolationtester spec file though, since there's a narrow window you need to race to. Hmm. I will look into this, thanks. 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, COMMIT, then UPDATE, all after the first session has taken its snapshot but before it tries to lock a given row. The code in EvalPlanQualFetch (reached from the HeapTupleUpdated case in ExecLockRow) finishes up waiting for the uncommitted transaction. I think that's the issue Andres and I patched for 9.3, but I don't know if it got committed. I'll need to have a look. A search in the archives for heap_lock_tuple and nowait might be informative. The difficulty of course will be testing all these racy cases reproducibly... Yep, especially as isolationtester can only really work at the statement level, and can only handle one blocked connection at a time. It's possible a helper extension could be used - set up some locks in shmem, register two sessions for different test roles within a given test to install appropriate hooks, wait until they're both blocked on the locks the hooks acquire, then release the locks. That might land up with hook points scattered everywhere, but they could be some pretty minimal macros at least. IMO that's a separate project, though. For this kind of testing I've tended to just set conditional breakpoints in gdb, wait until they trap, then once everything's lined up release the breakpoints in both sessions as simultaneously as possible. Not perfect, but has tended to work. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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, COMMIT, then UPDATE, all after the first session has taken its snapshot but before it tries to lock a given row. The code in EvalPlanQualFetch (reached from the HeapTupleUpdated case in ExecLockRow) finishes up waiting for the uncommitted transaction. Interesting -- perhaps this helps explain the deadlock issue reported in http://www.postgresql.org/message-id/cakrjmhdn+ghajnwqfhsotgp+7yn27zr79m99rcajmnazt5n...@mail.gmail.com I think I see how to teach EvalPlanQualFetch how to handle wait policies: for NOWAIT it should ereport (fixing a pre-existing bug (?)), and I guess it should handle SKIP LOCKED by returning NULL, similarly to the way it handles deleted rows, and of course in all cases passing the wait policy forward to heap_lock_tuple, which it eventually calls. Still looking at heap_lock_updated_tuple. 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 query to get its snapshot and then go to sleep before actually doing anything, by way of an advisory lock. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 realize this in review of f88d4cfc9. I think I will take that out instead of cluttering this patch with it. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 by another transaction).
* See comments for struct HeapUpdateFailureData for additional info.
With the patch as submitted, this struct is not filled in the
HeapTupleWouldBlock case. I'm not sure this is okay, though I admit the
only caller that passes LockWaitSkip doesn't care, so maybe we could
just ignore the issue (after properly modifying the comment). It seems
easy to add a LockBuffer() and goto failed rather than a return; that
would make that failure case conform to HeapTupleUpdated and
HeapTupleSelfUpdated. OTOH it might be pointless to worry about what
would be essentially dead code currently ...
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 updated tuple, then you either skip it because its
updater is running, or you return it because it's no longer running; and
if you return it, it is not possible for the updater to be locking the
updated version. However, what if there's a third transaction that
locked the updated version? It might be difficult to hit this case or
construct an isolationtester spec file though, since there's a narrow
window you need to race to.
I also pgindented.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 31073bc..5792036 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -109,8 +109,7 @@ static void MultiXactIdWait(MultiXactId multi, MultiXactStatus status, uint16 in
Relation rel, ItemPointer ctid, XLTW_Oper oper,
int *remaining);
static bool ConditionalMultiXactIdWait(MultiXactId multi, MultiXactStatus status,
- uint16 infomask, Relation rel, ItemPointer ctid,
- XLTW_Oper oper, int *remaining);
+ uint16 infomask, Relation rel, int *remaining);
static XLogRecPtr log_heap_new_cid(Relation relation, HeapTuple tup);
static HeapTuple ExtractReplicaIdentity(Relation rel, HeapTuple tup, bool key_modified,
bool *copy);
@@ -2818,7 +2817,7 @@ l1:
if (result == HeapTupleSelfUpdated)
hufd-cmax = HeapTupleHeaderGetCmax(tp.t_data);
else
- hufd-cmax = 0; /* for lack of an InvalidCommandId value */
+ hufd-cmax = InvalidCommandId;
UnlockReleaseBuffer(buffer);
if (have_tuple_lock)
UnlockTupleTuplock(relation, (tp.t_self), LockTupleExclusive);
@@ -3415,7 +3414,7 @@ l2:
if (result == HeapTupleSelfUpdated)
hufd-cmax = HeapTupleHeaderGetCmax(oldtup.t_data);
else
- hufd-cmax = 0; /* for lack of an InvalidCommandId value */
+ hufd-cmax = InvalidCommandId;
UnlockReleaseBuffer(buffer);
if (have_tuple_lock)
UnlockTupleTuplock(relation, (oldtup.t_self), *lockmode);
@@ -4222,22 +4221,27 @@ l3:
*/
if (!have_tuple_lock)
{
- if (wait_policy == LockWaitBlock)
+ switch (wait_policy)
{
-LockTupleTuplock(relation, tid, mode);
- }
- else if (wait_policy == LockWaitError)
- {
-if (!ConditionalLockTupleTuplock(relation, tid, mode))
- ereport(ERROR,
- (errcode(ERRCODE_LOCK_NOT_AVAILABLE),
- errmsg(could not obtain lock on row in relation \%s\,
- RelationGetRelationName(relation;
- }
- else /* wait_policy == LockWaitSkip */
- {
-if (!ConditionalLockTupleTuplock(relation, tid, mode))
- return HeapTupleWouldBlock;
+case LockWaitBlock:
+ LockTupleTuplock(relation, tid, mode);
+ break;
+case LockWaitSkip:
+ if (!ConditionalLockTupleTuplock(relation, tid, mode))
+ {
+ result = HeapTupleWouldBlock;
+ /* recovery code expects to have buffer lock held */
+ LockBuffer(*buffer, BUFFER_LOCK_EXCLUSIVE);
+ goto failed;
+ }
+ break;
+case LockWaitError:
+ if (!ConditionalLockTupleTuplock(relation, tid, mode))
+ ereport(ERROR,
+(errcode(ERRCODE_LOCK_NOT_AVAILABLE),
+ errmsg(could not obtain lock on row in relation \%s\,
+ RelationGetRelationName(relation;
+ break;
}
have_tuple_lock = true;
}
@@ -4441,34 +4445,34 @@ l3:
if (status = MultiXactStatusNoKeyUpdate)
elog(ERROR, invalid lock mode in heap_lock_tuple);
-/* wait for multixact to end */
-if (wait_policy == LockWaitBlock)
-{
- MultiXactIdWait((MultiXactId) xwait, status, infomask,
- relation, tuple-t_data-t_ctid,
- XLTW_Lock, NULL);
-}
-else if (wait_policy == LockWaitError)
-{
- if (!ConditionalMultiXactIdWait((MultiXactId) xwait,
- status, infomask, relation,
- tuple-t_data-t_ctid,
- XLTW_Lock,
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 that NOWAIT wins over the standard FOR UPDATE, but with NOWAIT, there's only a chance of an error, there's no chance of giving incorrect results. Another option is to throw an error at parse analysis time if there is a conflict in the specified locking policies, as in the above case. Are there cases in which it would make sense to have one clause trump the other? It seems reasonable to have NOWAIT trump regular FOR UPDATE (as it already does), since, as you say, there's chance of error being thrown at runtime, but not of incorrect result. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 thought the tests were a bit thin around NOWAIT then maybe that should be a separate patch? The isolation tester is new so we don't have nearly enough tests for it. Adding more meaningful tests is good even if they're unrelated to the patch at hand. I completely agree that some more isolation tests coverage would be a good thing. I just saw it as something not directly related to this feature, so thought it would be better as a separate patch. From my experience with the project, normally when I try to sneak something extra in, it either does not make the final commit, or gets added in a separate commit. Regards David Rowley
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 heap_lock_tuple. I will follow up with some more thorough isolation tests in the next week or so to cover the other two, and some other scenarios and interactions with other feature. Now with extra isolation tests so that the three different code branches that can skip rows are covered. I temporarily added some logging lines to double check that the expected branches are reached by each permutation while developing the specs. They change the output and are not part of the patch -- attaching separately. I've had a look over the isolation tests now and I can't see too much wrong, just a couple of typos... * skip-locked-2.spec # s2 againt skips because it can't acquired a multi-xact lock againt should be again also mixed use of multixact and multi-xact, probably would be better to stick to just 1. Also this file would probably be slightly easier to read with a new line after each permutation line. * skip_locked-3.spec # s3 skips to second record due to tuple lock held by s2 There's a missing the after skips to Also, won't the lock be held by s1 not s2? There's just a couple of other tiny things. * Some whitespace issues shown by git diff --check src/backend/parser/gram.y:9221: trailing whitespace. +opt_nowait_or_skip: src/backend/rewrite/rewriteHandler.c:65: trailing whitespace. + LockClauseStrength strength, LockWaitPolicy waitPolicy, * analyze.c The StaticAssertStmt's I think these should be removed. The only place where this behaviour can be changed is in lockwaitpolicy.h, I think it would be better to just strengthen the comment on the enum's definition. Perhaps something more along the lines of: Policy for what to do when a row lock cannot be obtained immediately. The enum values defined here have critical importance to how the parser treats multiple FOR UPDATE/SHARE statements in different nested levels of the query. Effectively if multiple locking clauses are defined in the query then the one with the highest enum value takes precedence over the others. 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. Good work Regards David Rowley
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 as seen in other places.
Also this file would probably be slightly easier to read with a new line
after each permutation line.
Done.
* skip_locked-3.spec
# s3 skips to second record due to tuple lock held by s2
There's a missing the after skips to
Fixed.
Also, won't the lock be held by s1 not s2?
s1 holds the row lock, and s2 holds the tuple lock (because it is at
the head of the queue waiting for the row lock). s3 skips because it
couldn't acquire the tuple lock held by s2. The other two specs are
about not being able to acquire a row lock and only need two sessions.
This one tests the code path when there is already a queue forming and
you can't even get the tuple lock, which requires an extra session. I
have updated the comment to make that clearer.
There's just a couple of other tiny things.
* Some whitespace issues shown by git diff --check
src/backend/parser/gram.y:9221: trailing whitespace.
+opt_nowait_or_skip:
src/backend/rewrite/rewriteHandler.c:65: trailing whitespace.
+
LockClauseStrength strength, LockWaitPolicy waitPolicy,
Fixed.
* analyze.c
The StaticAssertStmt's I think these should be removed. The only place where
this behaviour can be changed
is in lockwaitpolicy.h, I think it would be better to just strengthen the
comment on the enum's definition.
Removed.
Perhaps something more along the lines of:
Policy for what to do when a row lock cannot be obtained immediately.
The enum values defined here have critical importance to how the parser
treats multiple FOR UPDATE/SHARE statements in different nested levels of
the query. Effectively if multiple locking clauses are defined in the query
then the one with the highest enum value takes precedence over the others.
Added something along those lines.
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.
Good work
Thanks for all the guidance, I appreciate it! My review karma account
is now well overdrawn.
Best regards,
Thomas Munro
diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index 231dc6a..0469705 100644
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -45,7 +45,7 @@ SELECT [ ALL | DISTINCT [ ON ( replaceable class=parameterexpression/replac
[ LIMIT { replaceable class=parametercount/replaceable | ALL } ]
[ OFFSET replaceable class=parameterstart/replaceable [ ROW | ROWS ] ]
[ FETCH { FIRST | NEXT } [ replaceable class=parametercount/replaceable ] { ROW | ROWS } ONLY ]
-[ FOR { UPDATE | NO KEY UPDATE | SHARE | KEY SHARE } [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT ] [...] ]
+[ FOR { UPDATE | NO KEY UPDATE | SHARE | KEY SHARE } [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT | SKIP LOCKED ] [...] ]
phrasewhere replaceable class=parameterfrom_item/replaceable can be one of:/phrase
@@ -1283,7 +1283,7 @@ FETCH { FIRST | NEXT } [ replaceable class=parametercount/replaceable ] {
The locking clause has the general form
synopsis
-FOR replaceablelock_strength/ [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT ]
+FOR replaceablelock_strength/ [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT | SKIP LOCKED ]
/synopsis
where replaceablelock_strength/ can be one of
@@ -1359,11 +1359,17 @@ KEY SHARE
para
To prevent the operation from waiting for other transactions to commit,
-use the literalNOWAIT/ option. With literalNOWAIT/, the statement
-reports an error, rather than waiting, if a selected row
-cannot be locked immediately. Note that literalNOWAIT/ applies only
-to the row-level lock(s) mdash; the required literalROW SHARE/literal
-table-level lock is still taken in the ordinary way (see
+use either the literalNOWAIT/ or literalSKIP LOCKED/literal
+option. With literalNOWAIT/, the statement reports an error, rather
+than waiting, if a selected row cannot be locked immediately.
+With literalSKIP LOCKED/literal, any selected rows that cannot be
+immediately locked are skipped. Skipping locked rows provides an
+inconsistent view of the data, so this is not suitable for general purpose
+work, but can be used to avoid lock contention with multiple consumers
+accessing a queue-like table. Note that literalNOWAIT/
+and literalSKIP LOCKED/literal apply only to the row-level lock(s)
+mdash; the required literalROW SHARE/literal table-level lock is
+still taken in the ordinary way (see
xref linkend=mvcc). You can use
xref
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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. Good work Thanks for all the guidance, I appreciate it! My review karma account is now well overdrawn. Ok, then I have nothing more so it's time to pass this one along. 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 that NOWAIT wins over the standard FOR UPDATE, but with NOWAIT, there's only a chance of an error, there's no chance of giving incorrect results. I checked what Oracle did in this situation and I see that they completely disallow FOR UPDATE inside of views and subqueries. I could see an argument here that the outer most FOR UPDATE clause should be used, but I guess that ship has sailed when NOWAIT was added. Marking as ready for commiter. Regards David Rowley
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 were a bit thin around NOWAIT then maybe that should be a separate patch? The isolation tester is new so we don't have nearly enough tests for it. Adding more meaningful tests is good even if they're unrelated to the patch at hand. Here are my isolation tests for NOWAIT as a separate patch, independent of SKIP LOCKED. They cover the tuple lock, regular row lock and multixact row lock cases. I guess this might be called white box testing, since it usese knowledge of how to construct schedules that hit the three interesting code paths that trigger the error, even though you can't see from the output why the error was raised in each case without extra instrumentation (though it did cross my mind that it could be interesting at the very least for testing if the error message were different in each case). If there are no objections I will add this to the next commitfest. Best regards Thomas Munro diff --git a/src/test/isolation/expected/nowait-2.out b/src/test/isolation/expected/nowait-2.out new file mode 100644 index 000..6e24bbb --- /dev/null +++ b/src/test/isolation/expected/nowait-2.out @@ -0,0 +1,43 @@ +Parsed test spec with 2 sessions + +starting permutation: s1a s2a s2b s1b s2c +step s1a: SELECT * FROM foo FOR SHARE NOWAIT; +id data + +1 x +step s2a: SELECT * FROM foo FOR SHARE NOWAIT; +id data + +1 x +step s2b: SELECT * FROM foo FOR UPDATE NOWAIT; +ERROR: could not obtain lock on row in relation foo +step s1b: COMMIT; +step s2c: COMMIT; + +starting permutation: s2a s1a s2b s1b s2c +step s2a: SELECT * FROM foo FOR SHARE NOWAIT; +id data + +1 x +step s1a: SELECT * FROM foo FOR SHARE NOWAIT; +id data + +1 x +step s2b: SELECT * FROM foo FOR UPDATE NOWAIT; +ERROR: could not obtain lock on row in relation foo +step s1b: COMMIT; +step s2c: COMMIT; + +starting permutation: s2a s2b s1a s1b s2c +step s2a: SELECT * FROM foo FOR SHARE NOWAIT; +id data + +1 x +step s2b: SELECT * FROM foo FOR UPDATE NOWAIT; +id data + +1 x +step s1a: SELECT * FROM foo FOR SHARE NOWAIT; +ERROR: could not obtain lock on row in relation foo +step s1b: COMMIT; +step s2c: COMMIT; diff --git a/src/test/isolation/expected/nowait-3.out b/src/test/isolation/expected/nowait-3.out new file mode 100644 index 000..8444646 --- /dev/null +++ b/src/test/isolation/expected/nowait-3.out @@ -0,0 +1,17 @@ +Parsed test spec with 3 sessions + +starting permutation: s1a s2a s3a s1b s2b s3b +step s1a: SELECT * FROM foo FOR UPDATE; +id data + +1 x +step s2a: SELECT * FROM foo FOR UPDATE; waiting ... +step s3a: SELECT * FROM foo FOR UPDATE NOWAIT; +ERROR: could not obtain lock on row in relation foo +step s1b: COMMIT; +step s2a: ... completed +id data + +1 x +step s2b: COMMIT; +step s3b: COMMIT; diff --git a/src/test/isolation/expected/nowait.out b/src/test/isolation/expected/nowait.out new file mode 100644 index 000..a6343b4 --- /dev/null +++ b/src/test/isolation/expected/nowait.out @@ -0,0 +1,65 @@ +Parsed test spec with 2 sessions + +starting permutation: s1a s1b s2a s2b +step s1a: SELECT * FROM foo FOR UPDATE NOWAIT; +id data + +1 x +step s1b: COMMIT; +step s2a: SELECT * FROM foo FOR UPDATE NOWAIT; +id data + +1 x +step s2b: COMMIT; + +starting permutation: s1a s2a s1b s2b +step s1a: SELECT * FROM foo FOR UPDATE NOWAIT; +id data + +1 x +step s2a: SELECT * FROM foo FOR UPDATE NOWAIT; +ERROR: could not obtain lock on row in relation foo +step s1b: COMMIT; +step s2b: COMMIT; + +starting permutation: s1a s2a s2b s1b +step s1a: SELECT * FROM foo FOR UPDATE NOWAIT; +id data + +1 x +step s2a: SELECT * FROM foo FOR UPDATE NOWAIT; +ERROR: could not obtain lock on row in relation foo +step s2b: COMMIT; +step s1b: COMMIT; + +starting permutation: s2a s1a s1b s2b +step s2a: SELECT * FROM foo FOR UPDATE NOWAIT; +id data + +1 x +step s1a: SELECT * FROM foo FOR UPDATE NOWAIT; +ERROR: could not obtain lock on row in relation foo +step s1b: COMMIT; +step s2b: COMMIT; + +starting permutation: s2a s1a s2b s1b +step s2a: SELECT * FROM foo FOR UPDATE NOWAIT; +id data + +1 x
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 in the next week or so to
cover the other two, and some other scenarios and interactions with
other feature.
Now with extra isolation tests so that the three different code
branches that can skip rows are covered. I temporarily added some
logging lines to double check that the expected branches are reached
by each permutation while developing the specs. They change the
output and are not part of the patch -- attaching separately.
diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index 231dc6a..0469705 100644
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -45,7 +45,7 @@ SELECT [ ALL | DISTINCT [ ON ( replaceable class=parameterexpression/replac
[ LIMIT { replaceable class=parametercount/replaceable | ALL } ]
[ OFFSET replaceable class=parameterstart/replaceable [ ROW | ROWS ] ]
[ FETCH { FIRST | NEXT } [ replaceable class=parametercount/replaceable ] { ROW | ROWS } ONLY ]
-[ FOR { UPDATE | NO KEY UPDATE | SHARE | KEY SHARE } [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT ] [...] ]
+[ FOR { UPDATE | NO KEY UPDATE | SHARE | KEY SHARE } [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT | SKIP LOCKED ] [...] ]
phrasewhere replaceable class=parameterfrom_item/replaceable can be one of:/phrase
@@ -1283,7 +1283,7 @@ FETCH { FIRST | NEXT } [ replaceable class=parametercount/replaceable ] {
The locking clause has the general form
synopsis
-FOR replaceablelock_strength/ [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT ]
+FOR replaceablelock_strength/ [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT | SKIP LOCKED ]
/synopsis
where replaceablelock_strength/ can be one of
@@ -1359,11 +1359,17 @@ KEY SHARE
para
To prevent the operation from waiting for other transactions to commit,
-use the literalNOWAIT/ option. With literalNOWAIT/, the statement
-reports an error, rather than waiting, if a selected row
-cannot be locked immediately. Note that literalNOWAIT/ applies only
-to the row-level lock(s) mdash; the required literalROW SHARE/literal
-table-level lock is still taken in the ordinary way (see
+use either the literalNOWAIT/ or literalSKIP LOCKED/literal
+option. With literalNOWAIT/, the statement reports an error, rather
+than waiting, if a selected row cannot be locked immediately.
+With literalSKIP LOCKED/literal, any selected rows that cannot be
+immediately locked are skipped. Skipping locked rows provides an
+inconsistent view of the data, so this is not suitable for general purpose
+work, but can be used to avoid lock contention with multiple consumers
+accessing a queue-like table. Note that literalNOWAIT/
+and literalSKIP LOCKED/literal apply only to the row-level lock(s)
+mdash; the required literalROW SHARE/literal table-level lock is
+still taken in the ordinary way (see
xref linkend=mvcc). You can use
xref linkend=sql-lock
with the literalNOWAIT/ option first,
@@ -1386,14 +1392,14 @@ KEY SHARE
/para
para
-Multiple locking
-clauses can be written if it is necessary to specify different locking
-behavior for different tables. If the same table is mentioned (or
-implicitly affected) by more than one locking clause,
-then it is processed as if it was only specified by the strongest one.
-Similarly, a table is processed
-as literalNOWAIT/ if that is specified in any of the clauses
-affecting it.
+Multiple locking clauses can be written if it is necessary to specify
+different locking behavior for different tables. If the same table is
+mentioned (or implicitly affected) by more than one locking clause, then
+it is processed as if it was only specified by the strongest one.
+Similarly, a table is processed as literalNOWAIT/ if that is specified
+in any of the clauses affecting it. Otherwise, it is processed
+as literalSKIP LOCKED/literal if that is specified in any of the
+clauses affecting it.
/para
para
@@ -1930,9 +1936,9 @@ SELECT distributors.* WHERE distributors.name = 'Westward';
productnamePostgreSQL/productname allows it in any commandSELECT/
query as well as in sub-commandSELECT/s, but this is an extension.
The literalFOR NO KEY UPDATE/, literalFOR SHARE/ and
-literalFOR KEY SHARE/ variants,
-as well as the literalNOWAIT/ option,
-do not appear in the standard.
+literalFOR KEY SHARE/ variants, as well as the literalNOWAIT/
+and literalSKIP LOCKED/literal options, do not appear in the
+standard.
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 patch? The isolation tester is new so we don't have nearly enough tests for it. Adding more meaningful tests is good even if they're unrelated to the patch at hand. FWIW you can use configure --enable-coverage and make coverage-html to get coverage reports. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 src/include/datatype/aclmode.h? I was thinking src/include/utils/, actually, but maybe datatype/ would be a good choice. OTOH, what we've got in there now is just timestamp.h, and IIRC it was put there because it needed to be accessible from both frontend and backend contexts. That would not be true of aclmode.h, so perhaps aclmode.h doesn't belong there. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 enum are important due to the code in applyLockingClause(), but I see now instead that you've added some assert checks in applyLockingClause(), likely these should use Assert() rather than StaticAssertExpr(). 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 patch? In ExecLockRows(), is there a need to define the wait_policy variable now? It's just used once so you could probably just pass erm-waitPolicy directly to heap_lock_tuple(). I'm a bit confused at some of the code in heap_lock_tuple(). If I'm not wrong then after the line that does have_tuple_lock = true; it's never possible for have_tuple_lock to be false, but I see you've added checks to ensure we only unlock if have_tuple_lock is true. I'm thinking you probably did this because in the goto failed situation the check is done, but I was thinking that was perhaps put there in case a goto jump was added before have_tuple_lock is set to true. I'm wondering if it would be ok just to replace the test with an Assert() instead, or maybe just no check. Also, I'm just looking at some of the changes that you've done to function signatures... I see quite a few of them are now beyond 80 chars wide (see http://www.postgresql.org/docs/devel/static/source-format.html). Regards David Rowley
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 to have lost the comment which indicates that the values of the
enum are important due to the code in applyLockingClause(), but I see now
instead that you've added some assert checks in applyLockingClause(), likely
these should use Assert() rather than StaticAssertExpr().
Here's a new version with explicit numerical values and a comment in
lockwaitpolicy.h to explain that the order is important and point to
the relevant code. The assertions are about the relationship between
constant values known at compile time, so why would we want a runtime
assertion? I have changed it from StaticAssertExpr to
StaticAssertStmt though.
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 patch?
Since I was meddling with code that controls both NOWAIT and SKIP
LOCKED, I wanted to convince myself that I had not broken NOWAIT using
at least a basic smoke test . I suppose by the same logic I should
also wite isolation tests for default blocking FOR UPDATE... Ok, I've
taken nowait.spec out for now.
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 in the next week or so to
cover the other two, and some other scenarios and interactions with
other feature.
In ExecLockRows(), is there a need to define the wait_policy variable now?
It's just used once so you could probably just pass erm-waitPolicy directly
to heap_lock_tuple().
Fixed.
I'm a bit confused at some of the code in heap_lock_tuple(). If I'm not
wrong then after the line that does have_tuple_lock = true; it's never
possible for have_tuple_lock to be false, but I see you've added checks to
ensure we only unlock if have_tuple_lock is true. I'm thinking you probably
did this because in the goto failed situation the check is done, but I was
thinking that was perhaps put there in case a goto jump was added before
have_tuple_lock is set to true. I'm wondering if it would be ok just to
replace the test with an Assert() instead, or maybe just no check.
Right, I have removed the redundant conditionals.
Also, I'm just looking at some of the changes that you've done to function
signatures... I see quite a few of them are now beyond 80 chars wide (see
http://www.postgresql.org/docs/devel/static/source-format.html).
Fixed.
Best regards,
Thomas Munro
diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index 231dc6a..0469705 100644
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -45,7 +45,7 @@ SELECT [ ALL | DISTINCT [ ON ( replaceable class=parameterexpression/replac
[ LIMIT { replaceable class=parametercount/replaceable | ALL } ]
[ OFFSET replaceable class=parameterstart/replaceable [ ROW | ROWS ] ]
[ FETCH { FIRST | NEXT } [ replaceable class=parametercount/replaceable ] { ROW | ROWS } ONLY ]
-[ FOR { UPDATE | NO KEY UPDATE | SHARE | KEY SHARE } [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT ] [...] ]
+[ FOR { UPDATE | NO KEY UPDATE | SHARE | KEY SHARE } [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT | SKIP LOCKED ] [...] ]
phrasewhere replaceable class=parameterfrom_item/replaceable can be one of:/phrase
@@ -1283,7 +1283,7 @@ FETCH { FIRST | NEXT } [ replaceable class=parametercount/replaceable ] {
The locking clause has the general form
synopsis
-FOR replaceablelock_strength/ [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT ]
+FOR replaceablelock_strength/ [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT | SKIP LOCKED ]
/synopsis
where replaceablelock_strength/ can be one of
@@ -1359,11 +1359,17 @@ KEY SHARE
para
To prevent the operation from waiting for other transactions to commit,
-use the literalNOWAIT/ option. With literalNOWAIT/, the statement
-reports an error, rather than waiting, if a selected row
-cannot be locked immediately. Note that literalNOWAIT/ applies only
-to the row-level lock(s) mdash; the required literalROW SHARE/literal
-table-level lock is still taken in the ordinary way (see
+use either the literalNOWAIT/ or literalSKIP LOCKED/literal
+option. With literalNOWAIT/, the statement reports an error, rather
+than waiting, if a selected row cannot be locked immediately.
+With literalSKIP
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 = LockWaitSkip;
else /* erm-waitPolicy == RWP_NOWAIT */
wait_policy = LockWaitError;
Just after this code heap_lock_tuple() is called, and if that returns
HeapTupleWouldBlock, the code does a goto lnext, which then will repeat that
whole block again. I'm wondering why there's 2 enums that are for the same
thing? if you just had 1 then you'd not need this block of code at all, you
could just pass erm-waitPolicy to heap_lock_tuple().
True. Somewhere upthread I mentioned the difficulty I had deciding
how many enumerations were needed, for the various subsystems, ie
which headers and type they were allowed to share. [...]
I tried getting rid of the offending if-then-else enum conversion code
and replaced it with a simple assignment -- please see attached. I
also added compile time assertions that the enum values line up to
make that work, and are correctly ordered for use in that 'Max'
expression. Please let me know if you think this is an improvement or
an abomination.
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 suppose that the LockClauseWaitPolicy and RowWaitPolicy could at
least be merged into a single enum defined in nodes.h following the
example of CmdType, which is also used by both parsenodes.h and
plannnode.h, but do I detect a tiny hint of reluctance in its comment,
so put it here...?
(The attached patch also has a couple of trivial typo fixes in
documentation and comments).
Best regards,
Thomas Munro
diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index 231dc6a..0469705 100644
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -45,7 +45,7 @@ SELECT [ ALL | DISTINCT [ ON ( replaceable class=parameterexpression/replac
[ LIMIT { replaceable class=parametercount/replaceable | ALL } ]
[ OFFSET replaceable class=parameterstart/replaceable [ ROW | ROWS ] ]
[ FETCH { FIRST | NEXT } [ replaceable class=parametercount/replaceable ] { ROW | ROWS } ONLY ]
-[ FOR { UPDATE | NO KEY UPDATE | SHARE | KEY SHARE } [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT ] [...] ]
+[ FOR { UPDATE | NO KEY UPDATE | SHARE | KEY SHARE } [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT | SKIP LOCKED ] [...] ]
phrasewhere replaceable class=parameterfrom_item/replaceable can be one of:/phrase
@@ -1283,7 +1283,7 @@ FETCH { FIRST | NEXT } [ replaceable class=parametercount/replaceable ] {
The locking clause has the general form
synopsis
-FOR replaceablelock_strength/ [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT ]
+FOR replaceablelock_strength/ [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT | SKIP LOCKED ]
/synopsis
where replaceablelock_strength/ can be one of
@@ -1359,11 +1359,17 @@ KEY SHARE
para
To prevent the operation from waiting for other transactions to commit,
-use the literalNOWAIT/ option. With literalNOWAIT/, the statement
-reports an error, rather than waiting, if a selected row
-cannot be locked immediately. Note that literalNOWAIT/ applies only
-to the row-level lock(s) mdash; the required literalROW SHARE/literal
-table-level lock is still taken in the ordinary way (see
+use either the literalNOWAIT/ or literalSKIP LOCKED/literal
+option. With literalNOWAIT/, the statement reports an error, rather
+than waiting, if a selected row cannot be locked immediately.
+With literalSKIP LOCKED/literal, any selected rows that cannot be
+immediately locked are skipped. Skipping locked rows provides an
+inconsistent view of the data, so this is not suitable for general purpose
+work, but can be used to avoid lock contention with multiple consumers
+accessing a queue-like table. Note that literalNOWAIT/
+and literalSKIP LOCKED/literal apply only to the row-level lock(s)
+mdash; the required literalROW SHARE/literal table-level lock is
+still taken in the ordinary way (see
xref linkend=mvcc). You can use
xref linkend=sql-lock
with the literalNOWAIT/ option first,
@@ -1386,14 +1392,14 @@ KEY SHARE
/para
para
-Multiple locking
-clauses can be written if it is necessary to specify different locking
-behavior for different tables. If the same table is mentioned (or
-implicitly affected) by more than one locking clause,
-then it is processed as if it was only specified by the strongest one.
-Similarly, a table is processed
-as
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 the way I justify it in my head is something like, the 3 enums are for the same purpose, so having 3 exist all with different names is confusing and it makes the code harder to follow. So to fix that up I think, oh we can just give them all the same name... But then, how can be we be sure each definition matches the other 2? ... hmm, just merge it into one and put it somewhere that can be accessed from everywhere. Saying that I don't know what the project best practises are for locations for sharing such things, but if nothing exists then maybe this would be a good time to invent somewhere. Maybe someone with more experience can chime in and give advice on this? Regards David Rowley
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 AclMode, which is needed throughout the system and is currently declared in parsenodes.h. I can't say I've ever been particularly pleased with that arrangement though, since it forces inclusion of parsenodes.h in many places that might not otherwise have any interest in parse nodes. 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. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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
introduce one.
There is a precedent in the form of AclMode, which is needed throughout
the system and is currently declared in parsenodes.h. I can't say I've
ever been particularly pleased with that arrangement though, since it
forces inclusion of parsenodes.h in many places that might not otherwise
have any interest in parse nodes.
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.
Here is a new version of the patch with a single enum LockWaitPolicy
defined in utils/lockwaitpolicy.h.
Best regards,
Thomas Munro
diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index 231dc6a..0469705 100644
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -45,7 +45,7 @@ SELECT [ ALL | DISTINCT [ ON ( replaceable class=parameterexpression/replac
[ LIMIT { replaceable class=parametercount/replaceable | ALL } ]
[ OFFSET replaceable class=parameterstart/replaceable [ ROW | ROWS ] ]
[ FETCH { FIRST | NEXT } [ replaceable class=parametercount/replaceable ] { ROW | ROWS } ONLY ]
-[ FOR { UPDATE | NO KEY UPDATE | SHARE | KEY SHARE } [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT ] [...] ]
+[ FOR { UPDATE | NO KEY UPDATE | SHARE | KEY SHARE } [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT | SKIP LOCKED ] [...] ]
phrasewhere replaceable class=parameterfrom_item/replaceable can be one of:/phrase
@@ -1283,7 +1283,7 @@ FETCH { FIRST | NEXT } [ replaceable class=parametercount/replaceable ] {
The locking clause has the general form
synopsis
-FOR replaceablelock_strength/ [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT ]
+FOR replaceablelock_strength/ [ OF replaceable class=parametertable_name/replaceable [, ...] ] [ NOWAIT | SKIP LOCKED ]
/synopsis
where replaceablelock_strength/ can be one of
@@ -1359,11 +1359,17 @@ KEY SHARE
para
To prevent the operation from waiting for other transactions to commit,
-use the literalNOWAIT/ option. With literalNOWAIT/, the statement
-reports an error, rather than waiting, if a selected row
-cannot be locked immediately. Note that literalNOWAIT/ applies only
-to the row-level lock(s) mdash; the required literalROW SHARE/literal
-table-level lock is still taken in the ordinary way (see
+use either the literalNOWAIT/ or literalSKIP LOCKED/literal
+option. With literalNOWAIT/, the statement reports an error, rather
+than waiting, if a selected row cannot be locked immediately.
+With literalSKIP LOCKED/literal, any selected rows that cannot be
+immediately locked are skipped. Skipping locked rows provides an
+inconsistent view of the data, so this is not suitable for general purpose
+work, but can be used to avoid lock contention with multiple consumers
+accessing a queue-like table. Note that literalNOWAIT/
+and literalSKIP LOCKED/literal apply only to the row-level lock(s)
+mdash; the required literalROW SHARE/literal table-level lock is
+still taken in the ordinary way (see
xref linkend=mvcc). You can use
xref linkend=sql-lock
with the literalNOWAIT/ option first,
@@ -1386,14 +1392,14 @@ KEY SHARE
/para
para
-Multiple locking
-clauses can be written if it is necessary to specify different locking
-behavior for different tables. If the same table is mentioned (or
-implicitly affected) by more than one locking clause,
-then it is processed as if it was only specified by the strongest one.
-Similarly, a table is processed
-as literalNOWAIT/ if that is specified in any of the clauses
-affecting it.
+Multiple locking clauses can be written if it is necessary to specify
+different locking behavior for different tables. If the same table is
+mentioned (or implicitly affected) by more than one locking clause, then
+it is processed as if it was only specified by the strongest one.
+Similarly, a table is processed as literalNOWAIT/ if that is specified
+in any of the clauses affecting it. Otherwise, it is processed
+as literalSKIP LOCKED/literal if that is specified in any of the
+clauses affecting it.
/para
para
@@ -1930,9 +1936,9 @@ SELECT distributors.* WHERE distributors.name = 'Westward';
productnamePostgreSQL/productname allows it in any commandSELECT/
query as well as in sub-commandSELECT/s, but this is an extension.
The literalFOR NO KEY UPDATE/, literalFOR SHARE/ and
-
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 = LockWaitSkip; else /* erm-waitPolicy == RWP_NOWAIT */ wait_policy = LockWaitError; Just after this code heap_lock_tuple() is called, and if that returns HeapTupleWouldBlock, the code does a goto lnext, which then will repeat that whole block again. I'm wondering why there's 2 enums that are for the same thing? if you just had 1 then you'd not need this block of code at all, you could just pass erm-waitPolicy to heap_lock_tuple(). True. Somewhere upthread I mentioned the difficulty I had deciding how many enumerations were needed, for the various subsystems, ie which headers and type they were allowed to share. Then I put off working on this for so long that a nice example came along that showed me the way: the lock strength enums LockTupleMode (heapam.h) and RowMarkType (plannodes.h). The wait policy enums LockWaitPolicy (heapam.h) and RowWaitPolicy (plannodes.h) mirror them closely, and the same type of enumeration translation takes place in nodeLockRows.c immediately above the code you pasted. I don't have any more principled argument than monkey-see-monkey-do for that one... On reflection, I agree that this sucks, and I would like to unify the three new enums in the current patch (see below for recap) into one that can be passed between parser, planner, executor and heap access manager code as I think you are suggesting. My only question is: in which header should the enum be defined, that all those modules could include? Best regards, Thomas Munro Enumeration explosion recap: * parsenode.h defines enum LockClauseWaitPolicy, which is used in the structs LockClause and RowMarkClause, for use by the parser code * plannodes.h defines enum RowWaitPolicy, which is used in the structs PlanRowMark and ExecRowMark, for use by the planner and executor code (numbers coincide with LockClauseWaitPolicy) * heapam.h defines enum LockWaitPolicy, which is used as a parameter to heap_lock_tuple, for use by heap access code The parser produces LockClauseWaitPolicy values. InitPlan converts these to RowWaitPolicy values in execMain.c. Then nodeLockRows.c converts RowWaitPolicy values to LockWaitPolicy values (by if-then-else) so it can call heap_lock_tuple. This roughly mirrors what happens to lock strength information. The unpatched code simply passes a boolean 'nowait' flag around. An earlier version of my patch passed a pair of booleans around. Simon's independent patch[1] uses an int in the various node structs and the heap_lock_tuple function, and in execNode.h it has macros to give names to the values, and that is included by access/heapm.h. [1] http://www.postgresql.org/message-id/537610d5.3090...@2ndquadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 bit
side tracked with my own patches during this commitfest.
I'm really glad to see this patch is back again. I think it will be very
useful for processing queues. I could have made good use of it in my last
work, using it for sending unsent emails which were queued up in a table
in the database.
I've so far read over the patch and done only some brief tests of the
functionality.
Here's what I've picked up on so far:
* In heap_lock_tuple() there's a few places where you test the wait_policy,
but you're not always doing this in the same order. The previous code did
if (nolock) first each time, but since there's now 3 values I think if
(wait_policy == LockWaitError) should be first each time as likely this is
the most common case.
* The following small whitespace change can be removed in gram.y:
@@ -119,7 +119,6 @@ typedef struct PrivTarget
#define CAS_NOT_VALID 0x10
#define CAS_NO_INHERIT 0x20
-
#define parser_yyerror(msg) scanner_yyerror(msg, yyscanner)
#define parser_errposition(pos) scanner_errposition(pos, yyscanner)
* analyze.c. rc-waitPolicy = Max(rc-waitPolicy, waitPolicy);
I'm not quite sure I understand this yet, but I'm guessing the original
code was there so that something like:
SELECT * FROM (SELECT * FROM a FOR UPDATE) a FOR UPDATE NOWAIT;
Would give:
ERROR: could not obtain lock on row in relation a
So it seems that with the patch as you've defined in by the order of the
enum values in LockClauseWaitPolicy that SKIP LOCKED overrides FOR UPDATE.
I'm wondering if this is dangerous.
Should the following really skip locked tuples?
SELECT * FROM (SELECT * FROM a FOR UPDATE SKIP LOCKED) a FOR UPDATE LIMIT 1;
But on the other hand perhaps I've missed a discussion on this, if so then
I think the following comment should be updated to explain it all:
* We also consider that NOWAIT wins if it's specified both ways. This
* is a bit more debatable but raising an error doesn't seem helpful.
* (Consider for instance SELECT FOR UPDATE NOWAIT from a view that
* internally contains a plain FOR UPDATE spec.)
*
* plannodes.h - RowWaitPolicy waitPolicy; /* NOWAIT and SKIP LOCKED DATA
options */
Should be NOWAIT and SKIP LOCKED options since DATA has now been removed
from the syntax.
* nodeLockRow.c has extra space in if condition: else if (erm-waitPolicy
== RWP_SKIP )
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 = LockWaitSkip;
else /* erm-waitPolicy == RWP_NOWAIT */
wait_policy = LockWaitError;
Just after this code heap_lock_tuple() is called, and if that
returns HeapTupleWouldBlock, the code does a goto lnext, which then will
repeat that whole block again. I'm wondering why there's 2 enums that are
for the same thing? if you just had 1 then you'd not need this block of
code at all, you could just pass erm-waitPolicy to heap_lock_tuple().
* parsenodes.h comment does not meet project standards (
http://www.postgresql.org/docs/devel/static/source-format.html)
typedef enum LockClauseWaitPolicy
{
/* order is important (see applyLockingClause which takes the greatest
value when several wait policies have been specified), and values must
match RowWaitPolicy from plannodes.h */
* parsenode.h remove DATA from LockClauseWaitPolicy waitPolicy; /* NOWAIT
and SKIP LOCKED DATA */
I have noticed the /* TODO -- work out what needs to be released here */
comments in head_lock_tuple(), and perhaps the lack of cleaning up is what
is causing the following:
create table skiptest (id int primary key); insert into skiptest (id)
select x.x from generate_series(1,100) x(x);
Session 1: begin work; select * from skiptest for update limit 99;
Session 2: select * from skiptest for update skip locked limit 1;
WARNING: out of shared memory
ERROR: out of shared memory
HINT: You might need to increase max_locks_per_transaction.
Yet if I do:
session 1: begin work; select * from skiptest where id 1 for update;
session 2: select * from skiptest for update skip locked limit 1;
id
1
(1 row)
That test makes me think your todo comments are in the right place,
something is missing there for sure!
* plays about with patch for a bit *
I don't quite understand how heap_lock_tuple works, as if I debug session 2
from the above set of queries the first call
to ConditionalLockTupleTuplock() (heapam.c line 4236) succeeds where I'd
have thought it would fail, since session 1 should be locking this tuple?
Later at line 4527 on the line if (!ConditionalXactLockTableWait(xwait)),
it fails to grab the lock and returns HeapTupleWouldBlock. The above query
seems to work ok if I just
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 won't be any ambiguity there. We've had some problems in the past where allowing optional noise words resulted in grammar conflicts that made future features harder to add. See previous discussions about LOCK TABLE, wherein we almost went to the extreme of adding a completely separate ACQUIRE LOCK command. A lot of these things seem harmless when you first do them, and then later they seem less harmless. Anyway, +1 for the general idea of this feature. It's come up a number of times on this mailing list, and we've had customer requests for it, too. -- 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
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 [ WORK | TRANSACTION ] ... There won't be any ambiguity there. We've had some problems in the past where allowing optional noise words resulted in grammar conflicts that made future features harder to add. In this particular case, I'd be worried about whether we'd not end up having to fully reserve DATA in order to allow it to be optional here. That would be necessary if this clause could be followed immediately by an identifier, either now or in the future. That would be a mighty high price to pay for failing to make up our minds about which syntax to use. (How many tables out there do you think have data as a column name?) 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 AS-less column aliases when they are no keyword at all). If we're pulling syntax out of the air it'd be nice if we could avoid adding new keywords to the grammar. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 AS-less column aliases when they are no keyword at all). If we're pulling syntax out of the air it'd be nice if we could avoid adding new keywords to the grammar. How about some of these combinations of existing words: EXCLUDE LOCK NOWAIT EXCLUDE NOWAIT NEXT NOWAIT FOLLOWING NOWAIT DISCARD Of those I think I prefer NOWAIT EXCLUDE (perhaps with NOWAIT ABORT as a long version of the existing NOWAIT behaviour for contrast). Or adding just one new keyword: NOWAIT SKIP SKIP LOCK Regards, Thomas Munro
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 that whole syntax area is radically different to what we have, it isn't easy to maintain code compatibility. DB2 z/OS 10 provides SKIP LOCKED DATA clause to allow moving past already locked rows. That's fairly recent and I don't believe there will be many programs using that. DB2 UDB supports some complex functionality using DB2_SKIPINSERTED, DB2_EVALUNCOMMITTED and DB2_SKIPDELETED, all of which is complex and mostly exists for benchmarks, AFAICS. Oracle uses both SKIP LOCKED and NOWAIT. 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 that leaves us incompatible with Oracle, nor do I see any need to be compatible with both Oracle and DB2 since the latter is much less likely to gain us anything in practice. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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. SQLServer implements that using the table hint of READPAST. Since that whole syntax area is radically different to what we have, it isn't easy to maintain code compatibility. DB2 z/OS 10 provides SKIP LOCKED DATA clause to allow moving past already locked rows. That's fairly recent and I don't believe there will be many programs using that. DB2 UDB supports some complex functionality using DB2_SKIPINSERTED, DB2_EVALUNCOMMITTED and DB2_SKIPDELETED, all of which is complex and mostly exists for benchmarks, AFAICS. Oracle uses both SKIP LOCKED and NOWAIT. 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 that leaves us incompatible with Oracle, nor do I see any need to be compatible with both Oracle and DB2 since the latter is much less likely to gain us anything in practice. +1 Pavel -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 that leaves us incompatible with Oracle, nor do I see any need to be compatible with both Oracle and DB2 since the latter is much less likely to gain us anything in practice. +1. -- 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
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 the end of the SELECT statement, until all rows were deleted: BEGIN SELECT id, data FROM work WHERE status = 'NEW' LIMIT 1 FOR UPDATE -- if no rows returned, then finish UPDATE work SET status = 'WORK' WHERE id = $id COMMIT BEGIN DELETE FROM work WHERE id = $id COMMIT Here are the times taken to process all items, in elapsed seconds, on a slow laptop (i5 4 core with an SSD, with default postgresql.conf except for checkpoint_segments set to 300): | Threads | default | SKIP | | 1 | 26.78 | 27.02 | | 2 | 23.46 | 22.00 | | 3 | 22.02 | 14.83 | | 4 | 22.59 | 11.16 | | 5 | 22.37 | 9.05 | | 6 | 22.55 | 7.66 | | 7 | 22.46 | 6.69 | | 8 | 22.57 | 8.39 | | 9 | 22.40 | 8.38 | | 10 | 22.38 | 7.93 | | 11 | 22.43 | 6.86 | | 12 | 22.34 | 6.77 | I am not experienced at benchmarking and I don't claim that this particular workload or configuration is particularly sensible or representative of anything but it might give some idea of the motivation. Best regards, Thomas Munro
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 patch and a git tree when it's ready, hopefully tonight. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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-reading it I'd like to change waitMode into an enum, not
just some #defines .
Hope it's useful for comparison and ideas.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
From c9532344cdabcde4d1992659ec8be8ad4b0041ce Mon Sep 17 00:00:00 2001
From: Craig Ringer cr...@2ndquadrant.com
Date: Wed, 31 Jul 2013 18:37:46 +0800
Subject: [PATCH] implement FOR UPDATE SKIP LOCKED
---
src/backend/access/heap/heapam.c | 54 ---
src/backend/executor/execMain.c | 2 +-
src/backend/executor/nodeLockRows.c | 20 +++-
src/backend/nodes/copyfuncs.c | 6 ++--
src/backend/nodes/equalfuncs.c| 4 +--
src/backend/nodes/outfuncs.c | 6 ++--
src/backend/nodes/readfuncs.c | 2 +-
src/backend/optimizer/plan/planner.c | 4 +--
src/backend/optimizer/prep/prepsecurity.c | 8 ++---
src/backend/optimizer/prep/prepunion.c| 2 +-
src/backend/parser/analyze.c | 17 +-
src/backend/parser/gram.y | 23 -
src/backend/rewrite/rewriteHandler.c | 18 +--
src/backend/utils/adt/ruleutils.c | 4 ++-
src/include/access/heapam.h | 2 +-
src/include/nodes/execnodes.h | 5 ++-
src/include/nodes/parsenodes.h| 4 +--
src/include/nodes/plannodes.h | 2 +-
src/include/parser/analyze.h | 2 +-
src/include/parser/kwlist.h | 2 ++
20 files changed, 118 insertions(+), 69 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index b77c32c..8e7f2bf 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -56,6 +56,7 @@
#include catalog/namespace.h
#include miscadmin.h
#include pgstat.h
+#include nodes/execnodes.h
#include storage/bufmgr.h
#include storage/freespace.h
#include storage/lmgr.h
@@ -4081,7 +4082,7 @@ get_mxact_status_for_lock(LockTupleMode mode, bool is_update)
* cid: current command ID (used for visibility test, and stored into
* tuple's cmax if lock is successful)
* mode: indicates if shared or exclusive tuple lock is desired
- * nowait: if true, ereport rather than blocking if lock not available
+ * waitMode: mode describes handling of lock waits
* follow_updates: if true, follow the update chain to also lock descendant
* tuples.
*
@@ -4105,7 +4106,7 @@ get_mxact_status_for_lock(LockTupleMode mode, bool is_update)
*/
HTSU_Result
heap_lock_tuple(Relation relation, HeapTuple tuple,
-CommandId cid, LockTupleMode mode, bool nowait,
+CommandId cid, LockTupleMode mode, int waitMode,
bool follow_updates,
Buffer *buffer, HeapUpdateFailureData *hufd)
{
@@ -4208,16 +4209,21 @@ l3:
*/
if (!have_tuple_lock)
{
- if (nowait)
+ if (waitMode == WAITMODE_NORMAL)
+LockTupleTuplock(relation, tid, mode);
+ else
{
if (!ConditionalLockTupleTuplock(relation, tid, mode))
- ereport(ERROR,
+{
+ if (waitMode == WAITMODE_SKIP_LOCKED)
+ return result;
+ else if (waitMode == WAITMODE_NOWAIT)
+ ereport(ERROR,
(errcode(ERRCODE_LOCK_NOT_AVAILABLE),
- errmsg(could not obtain lock on row in relation \%s\,
- RelationGetRelationName(relation;
+ errmsg(could not obtain lock on row in relation \%s\,
+ RelationGetRelationName(relation;
+}
}
- else
-LockTupleTuplock(relation, tid, mode);
have_tuple_lock = true;
}
@@ -4419,21 +4425,26 @@ l3:
elog(ERROR, invalid lock mode in heap_lock_tuple);
/* wait for multixact to end */
-if (nowait)
+if (waitMode == WAITMODE_NORMAL)
+ MultiXactIdWait((MultiXactId) xwait, status, infomask,
+ relation, tuple-t_data-t_ctid,
+ XLTW_Lock, NULL);
+else
{
if (!ConditionalMultiXactIdWait((MultiXactId) xwait,
status, infomask, relation,
tuple-t_data-t_ctid,
XLTW_Lock, NULL))
- ereport(ERROR,
+ {
+ if (waitMode == WAITMODE_SKIP_LOCKED)
+ return result;
+ else if (waitMode == WAITMODE_NOWAIT)
+ ereport(ERROR,
(errcode(ERRCODE_LOCK_NOT_AVAILABLE),
errmsg(could not obtain lock on row in relation \%s\,
RelationGetRelationName(relation;
+ }
}
-else
- MultiXactIdWait((MultiXactId) xwait, status, infomask,
- relation, tuple-t_data-t_ctid,
- XLTW_Lock, NULL);
/* if there are updates, follow the update chain */
if (follow_updates
@@ -4479,17 +4490,22 @@ l3:
else
{
/* wait for regular
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 merge the isolation tests for it merged and sorted out, and after re-reading it I'd like to change waitMode into an enum, not just some #defines . Hope it's useful for comparison and ideas. Thank you! At first glance they're sort of similar which is reassuring. I'm especially interested in the buffer release semantics which I was confused about and will look into that (to resolve the TODO notes in my patch). 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. In my patch I have code that would give precedence to NOWAIT, though looking at it again it could be simpler. (One reviewer pointed out, that it should really be a single unified enum. In fact I have been a bit unsure about what scope such an enumeration should have in the application -- could it even be used in parser code? I tried to follow existing examples which is why I used #define macros in gram.y). From a bikeshed colour point of view: * I used SKIP LOCKED DATA like DB2, and Simon used SKIP LOCKED like Oracle, and I guess shorter is sweeter * I used the term wait_policy and an enum, Simon used waitMode and an int * I had noWait and skipLocked travelling separately in some places, Simon had a single parameter, which is much better Best regards, Thomas Munro
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 think that |= is correct there. It may be that no case can arise where you get the bogus value, but since in all other places the values are tested for equalty not as bit fields ( waitMode == NOWAIT not waitMode NOWAIT ) it doesn't make sense to |= it there. ? In my patch I have code that would give precedence to NOWAIT, though looking at it again it could be simpler. I agree; if NOWAIT is present anywhere it should be preferred to SKIP_LOCKED, as it's OK to apply NOWAIT where SKIP LOCKED appears, but possibly semantically incorrect to apply SKIP LOCKED where only NOWAIT was expected. (One reviewer pointed out, that it should really be a single unified enum. In fact I have been a bit unsure about what scope such an enumeration should have in the application -- could it even be used in parser code? I tried to follow existing examples which is why I used #define macros in gram.y). Not sure there. From a bikeshed colour point of view: * I used SKIP LOCKED DATA like DB2, and Simon used SKIP LOCKED like Oracle, and I guess shorter is sweeter 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 won't be any ambiguity there. * I used the term wait_policy and an enum, Simon used waitMode and an int I prefer an enum and intended to change Simon's patch but didn't have the time. I have some isolation tester and regression tests that are still to follow. * I had noWait and skipLocked travelling separately in some places, Simon had a single parameter, which is much better Yes, I strongly prefer that. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SKIP LOCKED DATA (work in progress)
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 some similar work a while ago, so I've cc'd him for his input. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
