Reduce lock levels others reloptions in ALTER TABLE
Hi all,
Some time ago we added [1]http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=47167b7907a802ed39b179c8780b76359468f076 the infrastructure to allow different lock
levels for relation options.
So per discussion [2]/messages/by-id/20150731022857.GC11473@alap3.anarazel.de the attached patch reduce lock levels down to
ShareUpdateExclusiveLock for:
- fastupdate
- fillfactor
- gin_pending_list_limit
- seq_page_cost
- random_page_cost
- n_distinct
- n_distinct_inherited
- buffering
Att,
[1]: http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=47167b7907a802ed39b179c8780b76359468f076
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=47167b7907a802ed39b179c8780b76359468f076
[2]: /messages/by-id/20150731022857.GC11473@alap3.anarazel.de
/messages/by-id/20150731022857.GC11473@alap3.anarazel.de
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
Timbira: http://www.timbira.com.br
Blog: http://fabriziomello.github.io
Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
Github: http://github.com/fabriziomello
Attachments:
reduce-locklevel-for-other-reloptions_v1.patchtext/x-diff; charset=US-ASCII; name=reduce-locklevel-for-other-reloptions_v1.patchDownload
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 86b9ae1..8128dd4 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -76,7 +76,7 @@ static relopt_bool boolRelOpts[] =
"fastupdate",
"Enables \"fast update\" feature for this GIN index",
RELOPT_KIND_GIN,
- AccessExclusiveLock
+ ShareUpdateExclusiveLock
},
true
},
@@ -100,7 +100,7 @@ static relopt_int intRelOpts[] =
"fillfactor",
"Packs table pages only to this percentage",
RELOPT_KIND_HEAP,
- AccessExclusiveLock
+ ShareUpdateExclusiveLock
},
HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100
},
@@ -109,7 +109,7 @@ static relopt_int intRelOpts[] =
"fillfactor",
"Packs btree index pages only to this percentage",
RELOPT_KIND_BTREE,
- AccessExclusiveLock
+ ShareUpdateExclusiveLock
},
BTREE_DEFAULT_FILLFACTOR, BTREE_MIN_FILLFACTOR, 100
},
@@ -118,7 +118,7 @@ static relopt_int intRelOpts[] =
"fillfactor",
"Packs hash index pages only to this percentage",
RELOPT_KIND_HASH,
- AccessExclusiveLock
+ ShareUpdateExclusiveLock
},
HASH_DEFAULT_FILLFACTOR, HASH_MIN_FILLFACTOR, 100
},
@@ -127,7 +127,7 @@ static relopt_int intRelOpts[] =
"fillfactor",
"Packs gist index pages only to this percentage",
RELOPT_KIND_GIST,
- AccessExclusiveLock
+ ShareUpdateExclusiveLock
},
GIST_DEFAULT_FILLFACTOR, GIST_MIN_FILLFACTOR, 100
},
@@ -136,7 +136,7 @@ static relopt_int intRelOpts[] =
"fillfactor",
"Packs spgist index pages only to this percentage",
RELOPT_KIND_SPGIST,
- AccessExclusiveLock
+ ShareUpdateExclusiveLock
},
SPGIST_DEFAULT_FILLFACTOR, SPGIST_MIN_FILLFACTOR, 100
},
@@ -250,7 +250,7 @@ static relopt_int intRelOpts[] =
"gin_pending_list_limit",
"Maximum size of the pending list for this GIN index, in kilobytes.",
RELOPT_KIND_GIN,
- AccessExclusiveLock
+ ShareUpdateExclusiveLock
},
-1, 64, MAX_KILOBYTES
},
@@ -297,7 +297,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
},
@@ -306,7 +306,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
},
@@ -315,7 +315,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
},
@@ -324,7 +324,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
},
@@ -339,7 +339,7 @@ static relopt_string stringRelOpts[] =
"buffering",
"Enables buffering build for this GiST index",
RELOPT_KIND_GIST,
- AccessExclusiveLock
+ ShareUpdateExclusiveLock
},
4,
false,
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 7c88ddc..3232cda 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -2065,19 +2065,19 @@ select * from my_locks order by 1;
commit;
begin; alter table alterlock set (fillfactor = 100);
select * from my_locks order by 1;
- relname | max_lockmode
------------+---------------------
- alterlock | AccessExclusiveLock
- pg_toast | AccessExclusiveLock
+ relname | max_lockmode
+-----------+--------------------------
+ alterlock | ShareUpdateExclusiveLock
+ pg_toast | ShareUpdateExclusiveLock
(2 rows)
commit;
begin; alter table alterlock reset (fillfactor);
select * from my_locks order by 1;
- relname | max_lockmode
------------+---------------------
- alterlock | AccessExclusiveLock
- pg_toast | AccessExclusiveLock
+ relname | max_lockmode
+-----------+--------------------------
+ alterlock | ShareUpdateExclusiveLock
+ pg_toast | ShareUpdateExclusiveLock
(2 rows)
commit;
@@ -2110,10 +2110,10 @@ rollback;
-- test that mixing options with different lock levels works as expected
begin; alter table alterlock set (autovacuum_enabled = off, fillfactor = 80);
select * from my_locks order by 1;
- relname | max_lockmode
------------+---------------------
- alterlock | AccessExclusiveLock
- pg_toast | AccessExclusiveLock
+ relname | max_lockmode
+-----------+--------------------------
+ alterlock | ShareUpdateExclusiveLock
+ pg_toast | ShareUpdateExclusiveLock
(2 rows)
commit;
On Mon, Feb 29, 2016 at 2:58 PM, Fabrízio de Royes Mello <
fabriziomello@gmail.com> wrote:
Hi all,
Some time ago we added [1] the infrastructure to allow different lock
levels for relation options.
So per discussion [2] the attached patch reduce lock levels down to
ShareUpdateExclusiveLock for:
- fastupdate
- fillfactor
- gin_pending_list_limit
- seq_page_cost
- random_page_cost
- n_distinct
- n_distinct_inherited
- bufferingAtt,
[1]
[2]
/messages/by-id/20150731022857.GC11473@alap3.anarazel.de
Added to the next commitfest:
https://commitfest.postgresql.org/9/558/
Regards,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
Timbira: http://www.timbira.com.br
Blog: http://fabriziomello.github.io
Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
Github: http://github.com/fabriziomello
On Mon, Feb 29, 2016 at 12:58 PM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:
Some time ago we added [1] the infrastructure to allow different lock levels
for relation options.So per discussion [2] the attached patch reduce lock levels down to
ShareUpdateExclusiveLock for:
- fastupdate
- fillfactor
- gin_pending_list_limit
- seq_page_cost
- random_page_cost
- n_distinct
- n_distinct_inherited
- buffering
I am of the opinion that there needs to be comments or a README or
some kind of documentation somewhere indicating the rationale for the
lock levels we choose here. Just changing it without analyzing why a
certain level is sufficient for safety and no higher than necessary
seems poor. And the reasoning should be documented for future
readers.
--
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
On 1 March 2016 at 19:00, Robert Haas <robertmhaas@gmail.com> wrote:
On Mon, Feb 29, 2016 at 12:58 PM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:Some time ago we added [1] the infrastructure to allow different lock
levels
for relation options.
So per discussion [2] the attached patch reduce lock levels down to
ShareUpdateExclusiveLock for:
- fastupdate
- fillfactor
- gin_pending_list_limit
- seq_page_cost
- random_page_cost
- n_distinct
- n_distinct_inherited
- bufferingI am of the opinion that there needs to be comments or a README or
some kind of documentation somewhere indicating the rationale for the
lock levels we choose here. Just changing it without analyzing why a
certain level is sufficient for safety and no higher than necessary
seems poor. And the reasoning should be documented for future
readers.
+1
The patch altered tests for fillfactor, so I extracted just that parameter
from the patch for commit, after adding comments and docs.
Fabrízio, you've been around long enough to know that patches need docs and
tests. And as Robert says, comments that show you've done the analysis to
show you know the patch is safe.
Some parts of this patch could be resubmitted in a later CF with some time
and attention spent on it, but it isn't in a good enough state for last CF.
--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services