Reduce lock levels for reloptions
Patch to reduce lock levels of various relation-level options,
following on from earlier work by Fabrizio, with additional work and
documentation by me.
Submitted to Sept CF, requested to start new thread to discuss
/messages/by-id/CA+TgmoYe5eDhjRodo3uOtVFGiDWwO2zGUp_mDHeSxuEqq-jS_A@mail.gmail.com
Earlier patch, dropped from 9.6
/messages/by-id/CANP8+j+0fuq2MWKvUsQTJFt8EF_z-Tyn-Q61J2A+VT2Uzuf-Rg@mail.gmail.com
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
reduce_lock_levels_incl_comments.v2.patchapplication/octet-stream; name=reduce_lock_levels_incl_comments.v2.patchDownload
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 42b4ea4..c506491 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -48,10 +48,46 @@
* (iii) add it to the appropriate options struct (perhaps StdRdOptions)
* (iv) add it to the appropriate handling routine (perhaps
* default_reloptions)
- * (v) don't forget to document the option
+ * (v) make sure the lock level is set correctly for that operation
+ * (vi) don't forget to document the option
*
* Note that we don't handle "oids" in relOpts because it is handled by
* interpretOidsOption().
+ *
+ * The default choice for any new option should be AccessExclusiveLock.
+ * In some cases the lock level can be reduced from there, but the lock
+ * level chosen should always conflict with itself to ensure that multiple
+ * changes aren't lost when we attempt concurrent changes.
+ * The choice of lock level depends completely upon how that parameter
+ * is used within the server, not upon how and when you'd like to change it.
+ * Safety first. Existing choices are documented here, and elsewhere in
+ * backend code where the parameters are used.
+ *
+ * In general, anything that affects the results obtained from a SELECT must be
+ * protected by AccessExclusiveLock.
+ *
+ * Autovacuum related parameters can be set at ShareUpdateExclusiveLock
+ * since they are only used by the AV procs and don't change anything
+ * currently executing.
+ *
+ * Fillfactor can be set because it applies only to subsequent changes made to
+ * data blocks, as documented in heapio.c
+ *
+ * n_distinct options can be set at ShareUpdateExclusiveLock because they
+ * are only used during ANALYZE, which uses a ShareUpdateExclusiveLock,
+ * so the ANALYZE will not be affected by in-flight changes. Changing those
+ * values has no affect until the next ANALYZE, so no need for stronger lock.
+ *
+ * Planner-related parameters can be set with ShareUpdateExclusiveLock because
+ * they only affect planning and not the correctness of the execution. Plans
+ * cannot be changed in mid-flight, so changes here could not easily result in
+ * new improved plans in any case. So we allow existing queries to continue
+ * and existing plans to survive, a small price to pay for allowing better
+ * plans to be introduced concurrently without interfering with users.
+ *
+ * Setting parallel_workers is safe, since it acts the same as
+ * max_parallel_workers_per_gather which is a USERSET parameter that doesn't
+ * affect existing plans or queries.
*/
static relopt_bool boolRelOpts[] =
@@ -267,7 +303,7 @@ static relopt_int intRelOpts[] =
"effective_io_concurrency",
"Number of simultaneous requests that can be handled efficiently by the disk subsystem.",
RELOPT_KIND_TABLESPACE,
- AccessExclusiveLock
+ ShareUpdateExclusiveLock
},
#ifdef USE_PREFETCH
-1, 0, MAX_IO_CONCURRENCY
@@ -280,7 +316,7 @@ static relopt_int intRelOpts[] =
"parallel_workers",
"Number of parallel processes that can be used per executor node for this relation.",
RELOPT_KIND_HEAP,
- AccessExclusiveLock
+ ShareUpdateExclusiveLock
},
-1, 0, 1024
},
@@ -314,7 +350,7 @@ static relopt_real realRelOpts[] =
"seq_page_cost",
"Sets the planner's estimate of the cost of a sequentially fetched disk page.",
RELOPT_KIND_TABLESPACE,
- AccessExclusiveLock
+ ShareUpdateExclusiveLock
},
-1, 0.0, DBL_MAX
},
@@ -323,7 +359,7 @@ static relopt_real realRelOpts[] =
"random_page_cost",
"Sets the planner's estimate of the cost of a nonsequentially fetched disk page.",
RELOPT_KIND_TABLESPACE,
- AccessExclusiveLock
+ ShareUpdateExclusiveLock
},
-1, 0.0, DBL_MAX
},
@@ -332,7 +368,7 @@ static relopt_real realRelOpts[] =
"n_distinct",
"Sets the planner's estimate of the number of distinct values appearing in a column (excluding child relations).",
RELOPT_KIND_ATTRIBUTE,
- AccessExclusiveLock
+ ShareUpdateExclusiveLock
},
0, -1.0, DBL_MAX
},
@@ -341,7 +377,7 @@ static relopt_real realRelOpts[] =
"n_distinct_inherited",
"Sets the planner's estimate of the number of distinct values appearing in a column (including child relations).",
RELOPT_KIND_ATTRIBUTE,
- AccessExclusiveLock
+ ShareUpdateExclusiveLock
},
0, -1.0, DBL_MAX
},
diff --git a/src/backend/utils/cache/spccache.c b/src/backend/utils/cache/spccache.c
index 9dcb80c..eafc00f 100644
--- a/src/backend/utils/cache/spccache.c
+++ b/src/backend/utils/cache/spccache.c
@@ -173,6 +173,10 @@ get_tablespace(Oid spcid)
/*
* get_tablespace_page_costs
* Return random and/or sequential page costs for a given tablespace.
+ *
+ * This value is not locked by the transaction, so this value may
+ * be changed while a SELECT that has used these values for planning
+ * is still executing.
*/
void
get_tablespace_page_costs(Oid spcid,
@@ -200,6 +204,13 @@ get_tablespace_page_costs(Oid spcid,
}
}
+/*
+ * get_tablespace_io_concurrency
+ *
+ * This value is not locked by the transaction, so this value may
+ * be changed while a SELECT that has used these values for planning
+ * is still executing.
+ */
int
get_tablespace_io_concurrency(Oid spcid)
{
On Sat, Feb 25, 2017 at 11:24 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
Patch to reduce lock levels of various relation-level options,
following on from earlier work by Fabrizio, with additional work and
documentation by me.
I have reviewed this patch and I think it looks reasonable. I think
that the behavior will be that overlapping transactions may or may not
pick up the new values depending on whether they already have a lock
on the relation and whether they happen to have a relcache flush for
some other reason, but new transactions started after the value is
altered via DDL should always see the new value, because they will
always do AcceptInvalidationMessages() after locking the relation, and
if they haven't yet picked up the value by that point, then they will
see it then. If that sounds right, maybe we should document someplace
in the SGML documentation that new values of reloptions which can be
changed without AEL might or might not be seen by overlapping
transactions; perhaps the lock levels for each reloption should also
be documented (since, as evidenced by this and previous patches,
people obviously do care about what those levels are).
--
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