DROP INDEX docs - explicit lock naming

Started by Greg Rychlewskialmost 5 years ago6 messages
#1Greg Rychlewski
greg.rychlewski@gmail.com
1 attachment(s)

Hi,

While reading the documentation for DROP INDEX[1]https://www.postgresql.org/docs/current/sql-dropindex.html, I noticed the lock was
described colloquially as an "exclusive" lock, which made me pause for a
second because it's the same name as the EXCLUSIVE table lock.

The attached patch explicitly states that an ACCESS EXCLUSIVE lock is
acquired.

[1]: https://www.postgresql.org/docs/current/sql-dropindex.html
https://www.postgresql.org/docs/current/sql-dropindex.html

Attachments:

v1-drop-index-doc.patchapplication/octet-stream; name=v1-drop-index-doc.patchDownload
commit f94c7a21d955dfc7970cacd9e060872afea8fff7
Author: Greg Rychlewski <greg.rychlewski@gmail.com>
Date:   Tue Mar 30 09:43:55 2021 -0400

    Be more explicit about lock naming in DROP INDEX docs

diff --git a/doc/src/sgml/ref/drop_index.sgml b/doc/src/sgml/ref/drop_index.sgml
index b6d2c2014f..b78fe89c07 100644
--- a/doc/src/sgml/ref/drop_index.sgml
+++ b/doc/src/sgml/ref/drop_index.sgml
@@ -45,7 +45,7 @@ DROP INDEX [ CONCURRENTLY ] [ IF EXISTS ] <replaceable class="parameter">name</r
      <para>
       Drop the index without locking out concurrent selects, inserts, updates,
       and deletes on the index's table.  A normal <command>DROP INDEX</command>
-      acquires an exclusive lock on the table, blocking other accesses until the
+      acquires an <literal>ACCESS EXCLUSIVE</literal> lock on the table, blocking other accesses until the
       index drop can be completed.  With this option, the command instead
       waits until conflicting transactions have completed.
      </para>
#2Michael Paquier
michael@paquier.xyz
In reply to: Greg Rychlewski (#1)
Re: DROP INDEX docs - explicit lock naming

On Tue, Mar 30, 2021 at 10:33:46AM -0400, Greg Rychlewski wrote:

While reading the documentation for DROP INDEX[1], I noticed the lock was
described colloquially as an "exclusive" lock, which made me pause for a
second because it's the same name as the EXCLUSIVE table lock.

The attached patch explicitly states that an ACCESS EXCLUSIVE lock is
acquired.

Indeed, this could be read as ACCESS SHARE being allowed, but that's
never the case for any of the index code paths, except if CONCURRENTLY
is involved. It is not the only place in the docs where we could do
more clarification. For instance, reindex.sgml mentions twice an
exclusive lock but that should be an access exclusive lock. To be
exact, I can spot 27 places under doc/ that could be improved. Such
changes depend on the surrounding context, of course.
--
Michael

#3Greg Rychlewski
greg.rychlewski@gmail.com
In reply to: Michael Paquier (#2)
1 attachment(s)
Re: DROP INDEX docs - explicit lock naming

Thanks for pointing that out. I've attached a new patch with several other
updates where I felt confident the docs were referring to an ACCESS
EXCLUSIVE lock.

On Tue, Mar 30, 2021 at 8:47 PM Michael Paquier <michael@paquier.xyz> wrote:

Show quoted text

On Tue, Mar 30, 2021 at 10:33:46AM -0400, Greg Rychlewski wrote:

While reading the documentation for DROP INDEX[1], I noticed the lock was
described colloquially as an "exclusive" lock, which made me pause for a
second because it's the same name as the EXCLUSIVE table lock.

The attached patch explicitly states that an ACCESS EXCLUSIVE lock is
acquired.

Indeed, this could be read as ACCESS SHARE being allowed, but that's
never the case for any of the index code paths, except if CONCURRENTLY
is involved. It is not the only place in the docs where we could do
more clarification. For instance, reindex.sgml mentions twice an
exclusive lock but that should be an access exclusive lock. To be
exact, I can spot 27 places under doc/ that could be improved. Such
changes depend on the surrounding context, of course.
--
Michael

Attachments:

v2-docs-access-exclusive-lock.patchapplication/octet-stream; name=v2-docs-access-exclusive-lock.patchDownload
commit 1559fb54bf006a79f92eb9086c2744e84c59daf6
Author: Greg Rychlewski <greg.rychlewski@gmail.com>
Date:   Tue Mar 30 09:43:55 2021 -0400

    Make documentation more explicit about when an ACCESS EXCLUSIVE lock is acquired.
    
    Several pages use the word "exclusive" colloquially to mean an ACCESS EXCLUSIVE lock.
    This can be confusing as there is another type of table lock named EXCLUSIVE.

diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index f073fbafd3..ec54885936 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -2630,7 +2630,7 @@ SELECT * FROM information WHERE group_id = 2 FOR UPDATE;
    definer function.)  Also, heavy concurrent use of row share locks on the
    referenced table could pose a performance problem, especially if updates
    of it are frequent.  Another solution, practical if updates of the
-   referenced table are infrequent, is to take an exclusive lock on the
+   referenced table are infrequent, is to take an <literal>ACCESS EXCLUSIVE</literal> lock on the
    referenced table when updating it, so that no concurrent transactions
    could be examining old row values.  Or one could just wait for all
    concurrent transactions to end after committing an update of the
diff --git a/doc/src/sgml/hstore.sgml b/doc/src/sgml/hstore.sgml
index db5779052a..8a85798f96 100644
--- a/doc/src/sgml/hstore.sgml
+++ b/doc/src/sgml/hstore.sgml
@@ -926,7 +926,7 @@ UPDATE tablename SET hstorecol = hstorecol || '';
 <programlisting>
 ALTER TABLE tablename ALTER hstorecol TYPE hstore USING hstorecol || '';
 </programlisting>
-   The <command>ALTER TABLE</command> method requires an exclusive lock on the table,
+   The <command>ALTER TABLE</command> method requires an <literal>ACCESS EXCLUSIVE</literal> lock on the table,
    but does not result in bloating the table with old row versions.
   </para>
 
diff --git a/doc/src/sgml/indexam.sgml b/doc/src/sgml/indexam.sgml
index ec5741df6d..08000d9321 100644
--- a/doc/src/sgml/indexam.sgml
+++ b/doc/src/sgml/indexam.sgml
@@ -980,7 +980,7 @@ amparallelrescan (IndexScanDesc scan);
    <literal>RowExclusiveLock</literal> when updating the index (including plain
    <command>VACUUM</command>).  Since these lock types do not conflict, the access
    method is responsible for handling any fine-grained locking it might need.
-   An exclusive lock on the index as a whole will be taken only during index
+   An <literal>ACCESS EXCLUSIVE</literal> lock on the index as a whole will be taken only during index
    creation, destruction, or <command>REINDEX</command>.
   </para>
 
diff --git a/doc/src/sgml/maintenance.sgml b/doc/src/sgml/maintenance.sgml
index 4d8ad754f8..a7d0977a1c 100644
--- a/doc/src/sgml/maintenance.sgml
+++ b/doc/src/sgml/maintenance.sgml
@@ -128,7 +128,7 @@
     <command>DELETE</command> will continue to function normally, though you
     will not be able to modify the definition of a table with commands such as
     <command>ALTER TABLE</command> while it is being vacuumed.)
-    <command>VACUUM FULL</command> requires exclusive lock on the table it is
+    <command>VACUUM FULL</command> requires an <literal>ACCESS EXCLUSIVE</literal> lock on the table it is
     working on, and therefore cannot be done in parallel with other use
     of the table.  Generally, therefore,
     administrators should strive to use standard <command>VACUUM</command> and
@@ -231,7 +231,7 @@
     or one of the table-rewriting variants of
     <link linkend="sql-altertable"><command>ALTER TABLE</command></link>.
     These commands rewrite an entire new copy of the table and build
-    new indexes for it.  All these options require exclusive lock.  Note that
+    new indexes for it.  All these options require an <literal>ACCESS EXCLUSIVE</literal> lock.  Note that
     they also temporarily use extra disk space approximately equal to the size
     of the table, since the old copies of the table and indexes can't be
     released until the new ones are complete.
diff --git a/doc/src/sgml/mvcc.sgml b/doc/src/sgml/mvcc.sgml
index 71dd403337..8d1e4eddc2 100644
--- a/doc/src/sgml/mvcc.sgml
+++ b/doc/src/sgml/mvcc.sgml
@@ -826,7 +826,7 @@ ERROR:  could not serialize access due to read/write dependencies among transact
     tables are not dropped or modified in incompatible ways while the
     command executes.  (For example, <command>TRUNCATE</command> cannot safely be
     executed concurrently with other operations on the same table, so it
-    obtains an exclusive lock on the table to enforce that.)
+    obtains an <literal>ACCESS EXCLUSIVE</literal> lock on the table to enforce that.)
    </para>
 
    <para>
diff --git a/doc/src/sgml/pgrowlocks.sgml b/doc/src/sgml/pgrowlocks.sgml
index 60e13393ea..f16fe05773 100644
--- a/doc/src/sgml/pgrowlocks.sgml
+++ b/doc/src/sgml/pgrowlocks.sgml
@@ -97,7 +97,7 @@ pgrowlocks(text) returns setof record
   <orderedlist>
    <listitem>
     <para>
-    If the table as a whole is exclusive-locked by someone else,
+    If the someone else has an <literal>ACCESS EXCLUSIVE</literal> lock on the table,
     <function>pgrowlocks</function> will be blocked.
     </para>
    </listitem>
diff --git a/doc/src/sgml/ref/drop_index.sgml b/doc/src/sgml/ref/drop_index.sgml
index b6d2c2014f..b78fe89c07 100644
--- a/doc/src/sgml/ref/drop_index.sgml
+++ b/doc/src/sgml/ref/drop_index.sgml
@@ -45,7 +45,7 @@ DROP INDEX [ CONCURRENTLY ] [ IF EXISTS ] <replaceable class="parameter">name</r
      <para>
       Drop the index without locking out concurrent selects, inserts, updates,
       and deletes on the index's table.  A normal <command>DROP INDEX</command>
-      acquires an exclusive lock on the table, blocking other accesses until the
+      acquires an <literal>ACCESS EXCLUSIVE</literal> lock on the table, blocking other accesses until the
       index drop can be completed.  With this option, the command instead
       waits until conflicting transactions have completed.
      </para>
diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml
index ff4dba8c36..b0e792dde5 100644
--- a/doc/src/sgml/ref/reindex.sgml
+++ b/doc/src/sgml/ref/reindex.sgml
@@ -282,10 +282,10 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN
    <command>REINDEX</command> is similar to a drop and recreate of the index
    in that the index contents are rebuilt from scratch.  However, the locking
    considerations are rather different.  <command>REINDEX</command> locks out writes
-   but not reads of the index's parent table.  It also takes an exclusive lock
+   but not reads of the index's parent table.  It also takes an <literal>ACCESS EXCLUSIVE</literal> lock
    on the specific index being processed, which will block reads that attempt
    to use that index.  In contrast, <command>DROP INDEX</command> momentarily takes
-   an exclusive lock on the parent table, blocking both writes and reads.  The
+   an <literal>ACCESS EXCLUSIVE</literal> lock on the parent table, blocking both writes and reads.  The
    subsequent <command>CREATE INDEX</command> locks out writes but not reads; since
    the index is not there, no read will attempt to use it, meaning that there
    will be no blocking but reads might be forced into expensive sequential
diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml
index bb1c0f8fb6..577261d6ab 100644
--- a/doc/src/sgml/ref/vacuum.sgml
+++ b/doc/src/sgml/ref/vacuum.sgml
@@ -83,7 +83,7 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
    specify parallel workers as zero.  <command>VACUUM FULL</command> rewrites
    the entire contents of the table into a new disk file with no extra space,
    allowing unused space to be returned to the operating system.  This form is
-   much slower and requires an exclusive lock on each table while it is being
+   much slower and requires an <literal>ACCESS EXCLUSIVE</literal> lock on each table while it is being
    processed.
   </para>
 
#4Michael Paquier
michael@paquier.xyz
In reply to: Greg Rychlewski (#3)
Re: DROP INDEX docs - explicit lock naming

On Tue, Mar 30, 2021 at 11:29:17PM -0400, Greg Rychlewski wrote:

Thanks for pointing that out. I've attached a new patch with several other
updates where I felt confident the docs were referring to an ACCESS
EXCLUSIVE lock.

Thanks, applied! I have reviewed the whole and there is one place in
vacuum.sgml that could switch "exclusive lock" to "SHARE UPDATE
EXCLUSIVE lock" but I have left that out as it does not bring more
clarity in the text. The change in indexam.sgml was partially wrong
as REINDEX CONCURRENTLY does not take an access exclusive lock, and I
have tweaked a bit the wording of pgrowlocks.sgml.
--
Michael

#5Greg Rychlewski
greg.rychlewski@gmail.com
In reply to: Michael Paquier (#4)
Re: DROP INDEX docs - explicit lock naming

Thanks! I apologize, I added a commitfest entry for this and failed to add
it to my message: https://commitfest.postgresql.org/33/3053/.

This is my first time submitting a patch and I'm not sure if it needs to be
deleted now or if you are supposed to add yourself as a committer.

On Thu, Apr 1, 2021 at 2:32 AM Michael Paquier <michael@paquier.xyz> wrote:

Show quoted text

On Tue, Mar 30, 2021 at 11:29:17PM -0400, Greg Rychlewski wrote:

Thanks for pointing that out. I've attached a new patch with several

other

updates where I felt confident the docs were referring to an ACCESS
EXCLUSIVE lock.

Thanks, applied! I have reviewed the whole and there is one place in
vacuum.sgml that could switch "exclusive lock" to "SHARE UPDATE
EXCLUSIVE lock" but I have left that out as it does not bring more
clarity in the text. The change in indexam.sgml was partially wrong
as REINDEX CONCURRENTLY does not take an access exclusive lock, and I
have tweaked a bit the wording of pgrowlocks.sgml.
--
Michael

#6Michael Paquier
michael@paquier.xyz
In reply to: Greg Rychlewski (#5)
Re: DROP INDEX docs - explicit lock naming

On Thu, Apr 01, 2021 at 08:26:50AM -0400, Greg Rychlewski wrote:

Thanks! I apologize, I added a commitfest entry for this and failed to add
it to my message: https://commitfest.postgresql.org/33/3053/.

This is my first time submitting a patch and I'm not sure if it needs to be
deleted now or if you are supposed to add yourself as a committer.

Thanks, I did not notice that. The entry has been updated as it
should.
--
Michael