Thread: NO WAIT ...
hi everyone ...
i have attached a patch implementing NO WAIT with the help of a GUC
variable.
documentation should be included as well.
it works nicely for me.
test=# begin;
BEGIN
test=# show wait_for_locks;
wait_for_locks
----------------
row share
(1 row)
test=# lock table x in exclusive mode;
LOCK TABLE
test=# commit;
COMMIT
test=# begin;
BEGIN
test=# -- somebody else has locked the table ...
test=# lock table x in exclusive mode;
ERROR: LockAcquire failed
[hs@fedora pgsql]$ difforig > nowait.patch
./doc/src/sgml/runtime.sgml
./src/backend/storage/lmgr/lock.c
./src/backend/utils/misc/guc.c
./src/backend/utils/misc/postgresql.conf.sample
./src/bin/psql/tab-complete.c
./src/include/storage/lock.h
./src/include/utils/guc.h
i hope this patch is ok.
if there are any modifications needed just drop me a line.
maybe i will have some spare time to implement "SELECT FOR UPDATE
NOWAIT" (SQL version). maybe this extension would make sense as well
because many people porting from oracle to pg would like that.
cheers,
Hans
--
Cybertec Geschwinde u Schoenig
Schoengrabern 134, A-2020 Hollabrunn, Austria
Tel: +43/2952/30706 or +43/664/233 90 75
www.cybertec.at, www.postgresql.at, kernel.cybertec.at
*** ./doc/src/sgml/runtime.sgml.orig 2004-02-17 12:06:23.000000000 +0100
--- ./doc/src/sgml/runtime.sgml 2004-02-17 13:10:26.000000000 +0100
***************
*** 2555,2560 ****
--- 2555,2578 ----
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><varname>wait_for_locks</varname> (<type>string</type>)</term>
+ <listitem>
+ <para>
+ By default an SQL statement has to wait until the desired locks for
+ processing the command can be acquired. In some cases this is not
+ the best thing to do. Therefore it is possible to tell PostgreSQL
+ which locks it is worth waiting for. The following kinds of locks /
+ settings are supported: all, access share, row share, row exclusive,
+ share update exclusive, share, share row exclusive, exclusive,
+ access exclusive. <quote>all</> is the default setting and means that
+ PostgreSQL will wait for all locks. If <quote>share</> is specified
+ PostgreSQL will not wait for locks which are higher or equal to a
+ share lock.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</sect2>
*** ./src/backend/storage/lmgr/lock.c.orig 2004-02-17 09:45:04.000000000 +0100
--- ./src/backend/storage/lmgr/lock.c 2004-02-17 11:33:13.000000000 +0100
***************
*** 36,41 ****
--- 36,42 ----
#include "access/xact.h"
#include "miscadmin.h"
#include "storage/proc.h"
+ #include "utils/guc.h"
#include "utils/memutils.h"
#include "utils/ps_status.h"
***************
*** 435,440 ****
--- 436,455 ----
locktag->objId.blkno, lock_mode_names[lockmode]);
#endif
+ /* check if NO WAIT has been enabled
+ the basic strategy is very simple: if the user has selected "all"
+ PostgreSQL will act normally. However, if a certain lock level has
+ been defined PostgreSQL won't wait for locks which are higher than
+ the specified level */
+ if (Wait_for_locks != 0)
+ {
+ /* now we have to adjust dontWait */
+ if (lockmode >= Wait_for_locks)
+ {
+ dontWait = true;
+ }
+ }
+
/* ???????? This must be changed when short term locks will be used */
locktag->lockmethodid = lockmethodid;
*** ./src/backend/utils/misc/guc.c.orig 2004-02-17 09:48:34.000000000 +0100
--- ./src/backend/utils/misc/guc.c 2004-02-17 14:42:10.000000000 +0100
***************
*** 47,52 ****
--- 47,53 ----
#include "storage/fd.h"
#include "storage/freespace.h"
#include "storage/lock.h"
+ #include "storage/lmgr.h"
#include "storage/proc.h"
#include "tcop/tcopprot.h"
#include "utils/array.h"
***************
*** 85,90 ****
--- 86,93 ----
static const char *assign_defaultxactisolevel(const char *newval,
bool doit, GucSource source);
+ static const char *assign_wait_for_locks(const char *newval,
+ bool doit, GucSource source);
static const char *assign_log_min_messages(const char *newval,
bool doit, GucSource source);
static const char *assign_client_min_messages(const char *newval,
***************
*** 132,137 ****
--- 135,141 ----
int client_min_messages = NOTICE;
int log_min_duration_statement = -1;
+ int Wait_for_locks = 0;
/*
***************
*** 149,154 ****
--- 153,159 ----
static char *client_encoding_string;
static char *datestyle_string;
static char *default_iso_level_string;
+ static char *wait_for_locks_string;
static char *locale_collate;
static char *locale_ctype;
static char *regex_flavor_string;
***************
*** 1515,1521 ****
&default_iso_level_string,
"read committed", assign_defaultxactisolevel, NULL
},
!
{
{"dynamic_library_path", PGC_SUSET, CLIENT_CONN_OTHER,
gettext_noop("Sets the path for dynamically loadable modules."),
--- 1520,1537 ----
&default_iso_level_string,
"read committed", assign_defaultxactisolevel, NULL
},
! {
! {"wait_for_locks", PGC_USERSET, CLIENT_CONN_STATEMENT,
! gettext_noop("Tells PostgreSQL whether to wait for locks or not."),
! gettext_noop("Allowed settings: "
! "\"all\", \"access share\", \"row share\", "
! "\"row exclusive\", \"share update exclusive\", "
! "\"share\", \"share row exclusive\", "
! "\"exclusive\", \"access exclusive\".")
! },
! &wait_for_locks_string,
! "all", assign_wait_for_locks, NULL
! },
{
{"dynamic_library_path", PGC_SUSET, CLIENT_CONN_OTHER,
gettext_noop("Sets the path for dynamically loadable modules."),
***************
*** 4457,4462 ****
--- 4473,4533 ----
#endif
+ /* in this function we will check if the string in the config-file
+ contains a correct lock level */
+ static const char *
+ assign_wait_for_locks(const char *newval, bool doit, GucSource source)
+ {
+ if (strcasecmp(newval, "all") == 0)
+ {
+ if (doit)
+ Wait_for_locks = NoLock;
+ }
+ else if (strcasecmp(newval, "access share") == 0)
+ {
+ if (doit)
+ Wait_for_locks = AccessShareLock;
+ }
+ else if (strcasecmp(newval, "row share") == 0)
+ {
+ if (doit)
+ Wait_for_locks = RowShareLock;
+ }
+ else if (strcasecmp(newval, "row exclusive") == 0)
+ {
+ if (doit)
+ Wait_for_locks = RowExclusiveLock;
+ }
+ else if (strcasecmp(newval, "share update exclusive") == 0)
+ {
+ if (doit)
+ Wait_for_locks = ShareUpdateExclusiveLock;
+ }
+ else if (strcasecmp(newval, "share") == 0)
+ {
+ if (doit)
+ Wait_for_locks = ShareLock;
+ }
+ else if (strcasecmp(newval, "share row exclusive") == 0)
+ {
+ if (doit)
+ Wait_for_locks = ShareRowExclusiveLock;
+ }
+ else if (strcasecmp(newval, "exclusive") == 0)
+ {
+ if (doit)
+ Wait_for_locks = ExclusiveLock;
+ }
+ else if (strcasecmp(newval, "access exclusive") == 0)
+ {
+ if (doit)
+ Wait_for_locks = AccessExclusiveLock;
+ }
+ else
+ return NULL;
+ return newval;
+ }
+
static const char *
assign_defaultxactisolevel(const char *newval, bool doit, GucSource source)
{
*** ./src/backend/utils/misc/postgresql.conf.sample.orig 2004-02-17 11:02:35.000000000 +0100
--- ./src/backend/utils/misc/postgresql.conf.sample 2004-02-17 13:38:14.000000000 +0100
***************
*** 248,253 ****
--- 248,257 ----
# LOCK MANAGEMENT
#---------------------------------------------------------------------------
+ #wait_for_locks = 'all' # allowed settings: all, access share,
+ # row share, row exclusive, share update
+ # exclusive, share, share row exclusive,
+ # exclusive, access exclusive
#deadlock_timeout = 1000 # in milliseconds
#max_locks_per_transaction = 64 # min 10, ~260*max_connections bytes each
*** ./src/bin/psql/tab-complete.c.orig 2004-02-17 11:51:19.000000000 +0100
--- ./src/bin/psql/tab-complete.c 2004-02-17 11:51:54.000000000 +0100
***************
*** 567,572 ****
--- 567,573 ----
"unix_socket_directory",
"unix_socket_group",
"unix_socket_permissions",
+ "wait_for_locks",
"wal_buffers",
"wal_debug",
"wal_sync_method",
*** ./src/include/storage/lock.h.orig 2004-02-17 10:12:37.000000000 +0100
--- ./src/include/storage/lock.h 2004-02-17 10:16:51.000000000 +0100
***************
*** 32,37 ****
--- 32,45 ----
extern int max_locks_per_xact;
+
+ /* PostgreSQL supports NO WAIT via GUC variables
+ therefore we need information telling us whether we want to wait for locks or
+ not. In addition to that we want to tell the system for which locks we want
+ to wait */
+ extern int Wait_for_locks;
+
+
#ifdef LOCK_DEBUG
extern int Trace_lock_oidmin;
extern bool Trace_locks;
*** ./src/include/utils/guc.h.orig 2004-02-17 10:35:02.000000000 +0100
--- ./src/include/utils/guc.h 2004-02-17 10:35:29.000000000 +0100
***************
*** 126,131 ****
--- 126,132 ----
extern int log_min_messages;
extern int client_min_messages;
extern int log_min_duration_statement;
+ extern int Wait_for_locks;
extern void SetConfigOption(const char *name, const char *value,
=?ISO-8859-1?Q?Hans-J=FCrgen_Sch=F6nig?= <postgres@cybertec.at> writes:
> i have attached a patch implementing NO WAIT with the help of a GUC
> variable.
I consider this patch incredibly dangerous, as it affects *every* lock
taken, including system internal lock acquisitions.
I think it might be reasonable to implement a no-wait option on explicit
LOCK TABLE commands, and perhaps we could do it for SELECT FOR UPDATE
as well. But it should not be done in a way that breaks internal lock
attempts.
Also, I don't care for the idea of a GUC variable affecting this.
See recent discussions about how changing fundamental semantics via
easily-changed GUC values is risky. If we're going to do it we should
add syntax to the LOCK command so that apps explicitly request it.
regards, tom lane
Tom Lane wrote: > =?ISO-8859-1?Q?Hans-J=FCrgen_Sch=F6nig?= <postgres@cybertec.at> writes: > > i have attached a patch implementing NO WAIT with the help of a GUC > > variable. > > I consider this patch incredibly dangerous, as it affects *every* lock > taken, including system internal lock acquisitions. > > I think it might be reasonable to implement a no-wait option on explicit > LOCK TABLE commands, and perhaps we could do it for SELECT FOR UPDATE > as well. But it should not be done in a way that breaks internal lock > attempts. > > Also, I don't care for the idea of a GUC variable affecting this. > See recent discussions about how changing fundamental semantics via > easily-changed GUC values is risky. If we're going to do it we should > add syntax to the LOCK command so that apps explicitly request it. There was discussion and I thought agreement that a GUC was best because we wouldn't have to add syntax to every command. I think the idea originally was that we were going to do nowait only on exclusive locks and not shared ones, which would hopefully reduce system lock cases where are usually shared ones. I imagine folks would want it on UPDATE, DELETE, and VACUUM FULL too, and that's why a GUC makes more sense, rather than littering the syntax with nowait controls. Also, I don't see this changing sematics like the regex flavor did. With that one, we actually had stored data in a table that wouldn't match a CHECK constraint. This isn't going to affect data validity, only query success. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Tom,
Yes, it can be dangerous. I am aware of that.
The problem with adding NO WAIT to specific commands is that is
inheritly unflexible. I think this is why the community has agreed on
implementing it based on GUC.
I have done some testing with a real world application. As far as I can
see it did not have an impact on other internals (at least not when used
cleverly).
SELECT FOR UPDATE NO WAIT should be added as well because it might be
useful to Oracle <-> compatibility.
Do you think it would help to reduce the GUCs flexibility by reducing
the lock levels a user is allowed to define?
Best regards,
Hans
Tom Lane wrote:
> =?ISO-8859-1?Q?Hans-J=FCrgen_Sch=F6nig?= <postgres@cybertec.at> writes:
>
>>i have attached a patch implementing NO WAIT with the help of a GUC
>>variable.
>
>
> I consider this patch incredibly dangerous, as it affects *every* lock
> taken, including system internal lock acquisitions.
>
> I think it might be reasonable to implement a no-wait option on explicit
> LOCK TABLE commands, and perhaps we could do it for SELECT FOR UPDATE
> as well. But it should not be done in a way that breaks internal lock
> attempts.
>
> Also, I don't care for the idea of a GUC variable affecting this.
> See recent discussions about how changing fundamental semantics via
> easily-changed GUC values is risky. If we're going to do it we should
> add syntax to the LOCK command so that apps explicitly request it.
>
> regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
--
Cybertec Geschwinde u Schoenig
Schoengrabern 134, A-2020 Hollabrunn, Austria
Tel: +43/2952/30706 or +43/664/233 90 75
www.cybertec.at, www.postgresql.at, kernel.cybertec.at
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> I imagine folks would want it on UPDATE, DELETE, and VACUUM FULL too,
Why? You can do a SELECT FOR UPDATE first and then you know that you
have the row lock. There's no need for any special handling of UPDATE
or DELETE. I don't see the applicability to VACUUM, either.
BTW, one idea I was thinking about was that a SELECT FOR UPDATE NOWAIT
behavior might simply not return the rows it couldn't acquire lock on,
instead of erroring out. Not sure if this would be more or less useful
than the error behavior, but I can definitely think of possible
applications for it.
> Also, I don't see this changing sematics like the regex flavor did.
You're kidding. This is a much more fundamental change of behavior than
whether some seldom-used regex features work. In particular, we know
that the regex behavior does not affect any other part of the system.
I do not think any equivalent safety claims can be made for random
hacking of whether LockAcquire succeeds or not.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > I imagine folks would want it on UPDATE, DELETE, and VACUUM FULL too, > > Why? You can do a SELECT FOR UPDATE first and then you know that you > have the row lock. There's no need for any special handling of UPDATE > or DELETE. I don't see the applicability to VACUUM, either. Why bother when you can do it without the SELECT FOR UPDATE? > BTW, one idea I was thinking about was that a SELECT FOR UPDATE NOWAIT > behavior might simply not return the rows it couldn't acquire lock on, > instead of erroring out. Not sure if this would be more or less useful > than the error behavior, but I can definitely think of possible > applications for it. > > > Also, I don't see this changing sematics like the regex flavor did. > > You're kidding. This is a much more fundamental change of behavior than No, I am not. > whether some seldom-used regex features work. In particular, we know > that the regex behavior does not affect any other part of the system. > I do not think any equivalent safety claims can be made for random > hacking of whether LockAcquire succeeds or not. It throws an error. I don't see how that could cause actual data corruption or invalid data. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
=?ISO-8859-1?Q?Hans-J=FCrgen_Sch=F6nig?= <postgres@cybertec.at> writes:
> The problem with adding NO WAIT to specific commands is that is
> inheritly unflexible. I think this is why the community has agreed on
> implementing it based on GUC.
I recall no such agreement ... when was this exactly? In any case
Bruce's recent complaints about regex_flavor have altered my opinions
about GUC variables a bit. They are bigger safety risks than they look,
especially ones that change semantics and are intended to be modified on
the fly.
> Do you think it would help to reduce the GUCs flexibility by reducing
> the lock levels a user is allowed to define?
I will vote against the patch no matter what, but I agree that it would
be less dangerous if it were confined to only apply to a limited set of
lock types.
regards, tom lane
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> Why? You can do a SELECT FOR UPDATE first and then you know that you
>> have the row lock. There's no need for any special handling of UPDATE
>> or DELETE. I don't see the applicability to VACUUM, either.
> Why bother when you can do it without the SELECT FOR UPDATE?
Because you want the extra feature?
> It throws an error. I don't see how that could cause actual data
> corruption or invalid data.
I am concerned about what behavior will stop working nicely when locks
that normally always succeed suddenly error out instead. Perhaps it
won't corrupt your database, but that doesn't mean that the behavior
will be pleasant.
As an example: the proposed patch is able to cause an error instead of
waiting for access-share locks. Suppose you actually turn that on, and
then try to call some function, and the resulting attempt to read
pg_proc errors out because someone was transiently holding a conflicting
lock. This means your application fails, randomly, and in
hard-to-reproduce ways. Not only would the failure or not-failure
depend on what other people were doing, it'd depend on whether you'd
already cached the function definition (if so, no lock would actually
get taken on pg_proc during the call).
I think a feature narrowly focused on suppressing waits for specific
locks (like the lock on a specific row that you're trying to update)
would be useful. Implementing something that affects *every* lock in
the system is nothing more nor less than a foot-gun, because you could
never be very certain what lock attempts would fail.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Tom Lane wrote: > >> Why? You can do a SELECT FOR UPDATE first and then you know that you > >> have the row lock. There's no need for any special handling of UPDATE > >> or DELETE. I don't see the applicability to VACUUM, either. > > > Why bother when you can do it without the SELECT FOR UPDATE? > > Because you want the extra feature? > > > It throws an error. I don't see how that could cause actual data > > corruption or invalid data. > > I am concerned about what behavior will stop working nicely when locks > that normally always succeed suddenly error out instead. Perhaps it > won't corrupt your database, but that doesn't mean that the behavior > will be pleasant. > > As an example: the proposed patch is able to cause an error instead of > waiting for access-share locks. Suppose you actually turn that on, and > then try to call some function, and the resulting attempt to read > pg_proc errors out because someone was transiently holding a conflicting > lock. This means your application fails, randomly, and in > hard-to-reproduce ways. Not only would the failure or not-failure > depend on what other people were doing, it'd depend on whether you'd > already cached the function definition (if so, no lock would actually > get taken on pg_proc during the call). > > I think a feature narrowly focused on suppressing waits for specific > locks (like the lock on a specific row that you're trying to update) > would be useful. Implementing something that affects *every* lock in > the system is nothing more nor less than a foot-gun, because you could > never be very certain what lock attempts would fail. The original idea I had was for the GUC variable to affect only exclusive locks. If we want more, we can add it later. I agree the extra GUC which controls the types of locks we will nowait for seems pretty useless. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Tom Lane wrote:
> =?ISO-8859-1?Q?Hans-J=FCrgen_Sch=F6nig?= <postgres@cybertec.at> writes:
>
>>The problem with adding NO WAIT to specific commands is that is
>>inheritly unflexible. I think this is why the community has agreed on
>>implementing it based on GUC.
>
>
> I recall no such agreement ... when was this exactly? In any case
> Bruce's recent complaints about regex_flavor have altered my opinions
> about GUC variables a bit. They are bigger safety risks than they look,
> especially ones that change semantics and are intended to be modified on
> the fly.
I thought there was an agreement because the GUC version is on the TODO
list. Anyway ...
>>Do you think it would help to reduce the GUCs flexibility by reducing
>>the lock levels a user is allowed to define?
>
>
> I will vote against the patch no matter what, but I agree that it would
> be less dangerous if it were confined to only apply to a limited set of
> lock types.
>
> regards, tom lane
Tom,
I think we should compromise.
We can restrict it to locks which are high enough or higher to make
SELECT FOR UPDATE work.
Of course it would have been nice to have full flexibility but I think
we can have almost the same benefit for lower risk.
How about it? Tom, Bruce - which types of locks do we allow?
I will change the patch then.
Maybe this is the best solution.
Regards,
Hans
--
Cybertec Geschwinde u Schoenig
Schoengrabern 134, A-2020 Hollabrunn, Austria
Tel: +43/2952/30706 or +43/664/233 90 75
www.cybertec.at, www.postgresql.at, kernel.cybertec.at
Hans-J�rgen Sch�nig wrote: > Tom Lane wrote: > > =?ISO-8859-1?Q?Hans-J=FCrgen_Sch=F6nig?= <postgres@cybertec.at> writes: > > > >>The problem with adding NO WAIT to specific commands is that is > >>inheritly unflexible. I think this is why the community has agreed on > >>implementing it based on GUC. > > > > > > I recall no such agreement ... when was this exactly? In any case > > Bruce's recent complaints about regex_flavor have altered my opinions > > about GUC variables a bit. They are bigger safety risks than they look, > > especially ones that change semantics and are intended to be modified on > > the fly. > > I thought there was an agreement because the GUC version is on the TODO > list. Anyway ... There was, but we have seen some concerns about GUC controlling too much since we added it, I think. > >>Do you think it would help to reduce the GUCs flexibility by reducing > >>the lock levels a user is allowed to define? > > > > > > I will vote against the patch no matter what, but I agree that it would > > be less dangerous if it were confined to only apply to a limited set of > > lock types. > > > > regards, tom lane > > > Tom, > > I think we should compromise. > We can restrict it to locks which are high enough or higher to make > SELECT FOR UPDATE work. > Of course it would have been nice to have full flexibility but I think > we can have almost the same benefit for lower risk. > How about it? Tom, Bruce - which types of locks do we allow? > I will change the patch then. > Maybe this is the best solution. Well, you already allow the user to control the level of locks he wants to timeout on, so it is merely an issue of setting the default for your second GUC. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073