On Tue, 3 Jun 2025 17:39:50 +0900
Yugo Nagata <nag...@sraoss.co.jp> wrote:
> On Tue, 27 May 2025 09:00:01 +0300
> Alexander Lakhin <exclus...@gmail.com> wrote:
> I know there are other scenarios where the same is raises and I agree that
> it would be better to consider a more global solution instead of addressing
> each of them. However, I am not sure that improving the error message for
> each case doesn't not make sense.
To address the remaining cases where DDL commands fail with the internal
error "ERROR: tuple concurrently updated" due to insufficient locking,
I would like to propose improving the error reporting to produce a more
appropriate and user-facing error message. This should make it easier for
users to understand the cause of the failure.
Patch 0003 improves the error message shown when concurrent updates to a
system catalog tuple occur, producing output like:
ERROR: operation failed due to a concurrent command
DETAIL: Another command modified the same object in a concurrent session.
Patches 0001 and 0002 are unchanged from v2, except for updated commit messages.
I believe these patches are still useful, as they allow the operation to
complete
successfully after waiting, or to behave appropriately when the target function
is dropped by another session during the wait.
Best regards,
Yugo Nagata
--
Yugo Nagata <nag...@sraoss.co.jp>
>From 4ab2d5f6b4651b6cdad47a170f53147bb7d8d5c2 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nag...@sraoss.co.jp>
Date: Thu, 5 Jun 2025 15:32:37 +0900
Subject: [PATCH v3 3/3] Improve error reporting for concurrent updates on
system catalog tuples
Previously, when multiple sessions attempted to modify the same system
catalog tuple concurrently due to insufficient locking, DDL commands could
fail with an internal error:
ERROR: tuple concurrently updated
This commit improves the behavior by reporting a more appropriate and
user-facing error message in such cases, making it easier for users to
understand the cause of the failure.
---
src/backend/access/heap/heapam.c | 34 ++++++++++++++++++++++++++++----
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 0dcd6ee817e..712b7ded6d2 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -3194,6 +3194,7 @@ simple_heap_delete(Relation relation, ItemPointer tid)
{
TM_Result result;
TM_FailureData tmfd;
+ bool is_catalog = IsCatalogRelation(relation);
result = heap_delete(relation, tid,
GetCurrentCommandId(true), InvalidSnapshot,
@@ -3211,11 +3212,23 @@ simple_heap_delete(Relation relation, ItemPointer tid)
break;
case TM_Updated:
- elog(ERROR, "tuple concurrently updated");
+ if (is_catalog)
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("operation failed due to a concurrent command"),
+ errdetail("Another command modified the same object in a concurrent session.")));
+ else
+ elog(ERROR, "tuple concurrently updated");
break;
case TM_Deleted:
- elog(ERROR, "tuple concurrently deleted");
+ if (is_catalog)
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("operation failed due to a concurrent command"),
+ errdetail("Another command dropped the same object in a concurrent session.")));
+ else
+ elog(ERROR, "tuple concurrently deleted");
break;
default:
@@ -4486,6 +4499,7 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup,
TM_Result result;
TM_FailureData tmfd;
LockTupleMode lockmode;
+ bool is_catalog = IsCatalogRelation(relation);
result = heap_update(relation, otid, tup,
GetCurrentCommandId(true), InvalidSnapshot,
@@ -4503,11 +4517,23 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup,
break;
case TM_Updated:
- elog(ERROR, "tuple concurrently updated");
+ if (is_catalog)
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("operation failed due to a concurrent command"),
+ errdetail("Another command modified the same object in a concurrent session.")));
+ else
+ elog(ERROR, "tuple concurrently updated");
break;
case TM_Deleted:
- elog(ERROR, "tuple concurrently deleted");
+ if (is_catalog)
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("operation failed due to a concurrent command"),
+ errdetail("Another command dropped the same object in a concurrent session.")));
+ else
+ elog(ERROR, "tuple concurrently deleted");
break;
default:
--
2.43.0
>From 56070e0ab5a73cc090f79850af40b418ddf53645 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nag...@sraoss.co.jp>
Date: Mon, 31 Mar 2025 20:17:07 +0900
Subject: [PATCH v3 2/3] Prevent internal error caused by concurrent ALTER
FUNCTION
Previously, concurrent ALTER FUNCTION commands could fail with an
internal error "tuple concurrently updated". This occurred
because multiple sessions attempted to modify the same catalog tuple
simultaneously.
To prevent this, ensure that an exclusive lock on the function object
is acquired earlier in the process.
Additionally, if the target function is dropped by another session while
waiting for the lock, an appropriate error is raised to indicate the
object no longer exists.
---
src/backend/commands/functioncmds.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 0335e982b31..5bb03153c0d 100644
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -61,6 +61,7 @@
#include "parser/parse_func.h"
#include "parser/parse_type.h"
#include "pgstat.h"
+#include "storage/lmgr.h"
#include "tcop/pquery.h"
#include "tcop/utility.h"
#include "utils/acl.h"
@@ -1383,9 +1384,21 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt)
ObjectAddressSet(address, ProcedureRelationId, funcOid);
+ /* Lock the function so nobody else can do anything with it. */
+ LockDatabaseObject(ProcedureRelationId, funcOid, 0, AccessExclusiveLock);
+
+ /*
+ * It is possible that by the time we acquire the lock on function,
+ * concurrent DDL has removed it. We can test this by checking the
+ * existence of function. We get the tuple again to avoid the risk
+ * of function definition getting changed.
+ */
tup = SearchSysCacheCopy1(PROCOID, ObjectIdGetDatum(funcOid));
- if (!HeapTupleIsValid(tup)) /* should not happen */
- elog(ERROR, "cache lookup failed for function %u", funcOid);
+ if (!HeapTupleIsValid(tup))
+ ereport(ERROR,
+ errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("function \"%s\" does not exist",
+ NameListToString(stmt->func->objname)));
procForm = (Form_pg_proc) GETSTRUCT(tup);
--
2.43.0
>From 6c11eca73b0b5b847edf5e1eb5840ee692b1e5fe Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nag...@sraoss.co.jp>
Date: Mon, 31 Mar 2025 18:46:58 +0900
Subject: [PATCH v3 1/3] Prevent internal error at concurrent CREATE OR REPLACE
FUNCTION
Previously, concurrent CREATE OR REPLACE FUNCTION commands could fail
with an internal error "tuple concurrently updated". This occurred
because multiple sessions attempted to modify the same catalog tuple
simultaneously.
To prevent this, ensure that an exclusive lock on the function object
is acquired earlier in the process.
Additionally, if the target function is dropped by another session while
waiting for the lock, a new function is created instead.
---
src/backend/catalog/pg_proc.c | 38 +++++++++++++++++++++++++++--------
1 file changed, 30 insertions(+), 8 deletions(-)
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index 5fdcf24d5f8..a35696480c7 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -34,6 +34,7 @@
#include "parser/parse_coerce.h"
#include "pgstat.h"
#include "rewrite/rewriteHandler.h"
+#include "storage/lmgr.h"
#include "tcop/pquery.h"
#include "tcop/tcopprot.h"
#include "utils/acl.h"
@@ -383,24 +384,45 @@ ProcedureCreate(const char *procedureName,
tupDesc = RelationGetDescr(rel);
/* Check for pre-existing definition */
- oldtup = SearchSysCache3(PROCNAMEARGSNSP,
- PointerGetDatum(procedureName),
- PointerGetDatum(parameterTypes),
- ObjectIdGetDatum(procNamespace));
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
if (HeapTupleIsValid(oldtup))
{
/* There is one; okay to replace it? */
Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
- Datum proargnames;
- bool isnull;
- const char *dropcmd;
if (!replace)
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_FUNCTION),
errmsg("function \"%s\" already exists with same argument types",
procedureName)));
+
+ /* Lock the function so nobody else can do anything with it. */
+ LockDatabaseObject(ProcedureRelationId, oldproc->oid, 0, AccessExclusiveLock);
+
+ /*
+ * It is possible that by the time we acquire the lock on function,
+ * concurrent DDL has removed it. We can test this by checking the
+ * existence of function. We get the tuple again to avoid the risk
+ * of function definition getting changed.
+ */
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
+ }
+
+ if (HeapTupleIsValid(oldtup))
+ {
+
+ Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
+ Datum proargnames;
+ bool isnull;
+ const char *dropcmd;
+
if (!object_ownercheck(ProcedureRelationId, oldproc->oid, proowner))
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_FUNCTION,
procedureName);
@@ -585,7 +607,7 @@ ProcedureCreate(const char *procedureName,
tup = heap_modify_tuple(oldtup, tupDesc, values, nulls, replaces);
CatalogTupleUpdate(rel, &tup->t_self, tup);
- ReleaseSysCache(oldtup);
+ heap_freetuple(oldtup);
is_update = true;
}
else
--
2.43.0
