Thread: DROP INDEX docs - explicit lock naming
Hi,
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.
[1]
Attachment
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
Attachment
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:
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
Attachment
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
Attachment
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:
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
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