Locks on unlogged tables are locked?!

Started by Laurenz Albeover 8 years ago5 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

won't retrysuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t38750
psql -h localhost -U postgres

Built from patchset v1 (message #1), July 27, 2026 at 08:24 PM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t38750_1 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t38750_1 && git checkout t38750_1

Patchset v1 (message #1) is on t38750_1

Jump to latest
#1Laurenz Albe
laurenz.albe@cybertec.at

While looking at this:
https://stackoverflow.com/q/50413623/6464308
I realized that "LOCK TABLE <unlogged table>" puts a
Standby/LOCK into the WAL stream, which causes a log flush
at COMMIT time.

That hurts performance, and I doubt that it is necessary.
At any rate, DROP TABLE on an unlogged table logs nothing.

The attached patch would take care of it, but I'm not sure
if that's the right place to check.

Yours,
Laurenz Albe

Attachments:

t38750_1
0001-Don-t-log-locks-on-unlogged-tables.patchtext/x-patch; charset=UTF-8; name=0001-Don-t-log-locks-on-unlogged-tables.patchDownload+21-8
#2Bruce Momjian
bruce@momjian.us
In reply to: Laurenz Albe (#1)
Re: Locks on unlogged tables are locked?!

Uh, was this ever addressed? I don't see the patch applied or the code
in this area modified.

---------------------------------------------------------------------------

On Thu, May 24, 2018 at 04:33:11PM +0200, Laurenz Albe wrote:

While looking at this:
https://stackoverflow.com/q/50413623/6464308
I realized that "LOCK TABLE <unlogged table>" puts a
Standby/LOCK into the WAL stream, which causes a log flush
at COMMIT time.

That hurts performance, and I doubt that it is necessary.
At any rate, DROP TABLE on an unlogged table logs nothing.

The attached patch would take care of it, but I'm not sure
if that's the right place to check.

Yours,
Laurenz Albe

From d474e7b41298944e43bb9141eb33adbdd9ea1098 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.albe@cybertec.at>
Date: Tue, 22 May 2018 18:13:31 +0200
Subject: [PATCH] Don't log locks on unlogged tables

---
src/backend/storage/lmgr/lock.c | 28 +++++++++++++++++++++-------
1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index dc3d8d9817..70cac47ab3 100644
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -37,6 +37,7 @@
#include "access/twophase_rmgr.h"
#include "access/xact.h"
#include "access/xlog.h"
+#include "catalog/pg_class.h"
#include "miscadmin.h"
#include "pg_trace.h"
#include "pgstat.h"
@@ -47,6 +48,7 @@
#include "storage/standby.h"
#include "utils/memutils.h"
#include "utils/ps_status.h"
+#include "utils/rel.h"
#include "utils/resowner_private.h"
@@ -1041,13 +1043,25 @@ LockAcquireExtended(const LOCKTAG *locktag,
*/
if (log_lock)
{
-		/*
-		 * Decode the locktag back to the original values, to avoid sending
-		 * lots of empty bytes with every message.  See lock.h to check how a
-		 * locktag is defined for LOCKTAG_RELATION
-		 */
-		LogAccessExclusiveLock(locktag->locktag_field1,
-							   locktag->locktag_field2);
+		bool unlogged_rel = false;
+
+		if (locktag->locktag_type == LOCKTAG_RELATION)
+		{
+			Relation r = RelationIdGetRelation(locktag->locktag_field2);
+			unlogged_rel = r->rd_rel->relpersistence == RELPERSISTENCE_UNLOGGED;
+			RelationClose(r);
+		}
+
+		if (!unlogged_rel)
+		{
+			/*
+			 * Decode the locktag back to the original values, to avoid sending
+			 * lots of empty bytes with every message.  See lock.h to check how a
+			 * locktag is defined for LOCKTAG_RELATION
+			 */
+			LogAccessExclusiveLock(locktag->locktag_field1,
+								   locktag->locktag_field2);
+		}
}

return LOCKACQUIRE_OK;
--
2.14.3

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com

Only you can decide what is important to you.

#3Robert Haas
robertmhaas@gmail.com
In reply to: Bruce Momjian (#2)
Re: Locks on unlogged tables are locked?!

On Tue, Nov 21, 2023 at 11:49 AM Bruce Momjian <bruce@momjian.us> wrote:

Uh, was this ever addressed? I don't see the patch applied or the code
in this area modified.

I never saw this email originally, but I don't think I believe
Laurenz's argument. Are all AEL-requiring operations on unlogged
tables safe to perform on a standby without AEL? I believe I would be
quite surprised if that turned out to be true.

--
Robert Haas
EDB: http://www.enterprisedb.com

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#2)
Re: Locks on unlogged tables are locked?!

Bruce Momjian <bruce@momjian.us> writes:

Uh, was this ever addressed? I don't see the patch applied or the code
in this area modified.

This patch as-is would surely be disastrous: having LockAcquire
try to open the relcache entry for the thing we're trying to lock
is going to be circular in at least some cases. I'm not convinced
that there's a problem worth solving here, but if there is, it'd
have to be done in some other way.

regards, tom lane

#5Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#4)
Re: Locks on unlogged tables are locked?!

On Tue, Nov 21, 2023 at 01:16:19PM -0500, Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Uh, was this ever addressed? I don't see the patch applied or the code
in this area modified.

This patch as-is would surely be disastrous: having LockAcquire
try to open the relcache entry for the thing we're trying to lock
is going to be circular in at least some cases. I'm not convinced
that there's a problem worth solving here, but if there is, it'd
have to be done in some other way.

Thank you, and Robert, for the feedback.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com

Only you can decide what is important to you.