From f51b34b3eddbc501063f7b8ac470d26ce4e18a48 Mon Sep 17 00:00:00 2001 From: Noah Misch Date: Tue, 24 Sep 2024 15:25:18 -0700 Subject: For inplace update durability, make heap_update() callers wait. The previous commit fixed some ways of losing an inplace update. It remained possible to lose one when a backend working toward a heap_update() copied a tuple into memory just before inplace update of that tuple. In catalogs eligible for inplace update, use LOCKTAG_TUPLE to govern admission to the steps of copying an old tuple, modifying it, and issuing heap_update(). This includes MERGE commands. To avoid changing most of the pg_class DDL, don't require LOCKTAG_TUPLE when holding a relation lock sufficient to exclude inplace updaters. Back-patch to v12 (all supported versions). In v13 and v12, "UPDATE pg_class" or "UPDATE pg_database" can still lose an inplace update. The v14+ UPDATE fix needs commit 86dc90056dfdbd9d1b891718d2e5614e3e432f35, and it wasn't worth reimplementing that fix without such infrastructure. Reviewed by Nitin Motiani and (in earlier versions) Heikki Linnakangas. Discussion: https://postgr.es/m/20231027214946.79.nmisch@google.com --- src/backend/commands/dbcommands.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'src/backend/commands/dbcommands.c') diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index d3c7cae6b2f..57e342a61e8 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -1053,6 +1053,7 @@ RenameDatabase(const char *oldname, const char *newname) { Oid db_id; HeapTuple newtup; + ItemPointerData otid; Relation rel; int notherbackends; int npreparedxacts; @@ -1124,11 +1125,13 @@ RenameDatabase(const char *oldname, const char *newname) errdetail_busy_db(notherbackends, npreparedxacts))); /* rename */ - newtup = SearchSysCacheCopy1(DATABASEOID, ObjectIdGetDatum(db_id)); + newtup = SearchSysCacheLockedCopy1(DATABASEOID, ObjectIdGetDatum(db_id)); if (!HeapTupleIsValid(newtup)) elog(ERROR, "cache lookup failed for database %u", db_id); + otid = newtup->t_self; namestrcpy(&(((Form_pg_database) GETSTRUCT(newtup))->datname), newname); - CatalogTupleUpdate(rel, &newtup->t_self, newtup); + CatalogTupleUpdate(rel, &otid, newtup); + UnlockTuple(rel, &otid, InplaceUpdateTupleLock); InvokeObjectPostAlterHook(DatabaseRelationId, db_id, 0); @@ -1372,6 +1375,7 @@ movedb(const char *dbname, const char *tblspcname) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_DATABASE), errmsg("database \"%s\" does not exist", dbname))); + LockTuple(pgdbrel, &oldtuple->t_self, InplaceUpdateTupleLock); MemSet(new_record, 0, sizeof(new_record)); MemSet(new_record_nulls, false, sizeof(new_record_nulls)); @@ -1384,6 +1388,7 @@ movedb(const char *dbname, const char *tblspcname) new_record, new_record_nulls, new_record_repl); CatalogTupleUpdate(pgdbrel, &oldtuple->t_self, newtuple); + UnlockTuple(pgdbrel, &oldtuple->t_self, InplaceUpdateTupleLock); InvokeObjectPostAlterHook(DatabaseRelationId, db_id, 0); @@ -1620,6 +1625,7 @@ AlterDatabase(ParseState *pstate, AlterDatabaseStmt *stmt, bool isTopLevel) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_DATABASE), errmsg("database \"%s\" does not exist", stmt->dbname))); + LockTuple(rel, &tuple->t_self, InplaceUpdateTupleLock); datform = (Form_pg_database) GETSTRUCT(tuple); dboid = datform->oid; @@ -1673,6 +1679,7 @@ AlterDatabase(ParseState *pstate, AlterDatabaseStmt *stmt, bool isTopLevel) newtuple = heap_modify_tuple(tuple, RelationGetDescr(rel), new_record, new_record_nulls, new_record_repl); CatalogTupleUpdate(rel, &tuple->t_self, newtuple); + UnlockTuple(rel, &tuple->t_self, InplaceUpdateTupleLock); InvokeObjectPostAlterHook(DatabaseRelationId, dboid, 0); @@ -1783,6 +1790,8 @@ AlterDatabaseOwner(const char *dbname, Oid newOwnerId) (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied to change owner of database"))); + LockTuple(rel, &tuple->t_self, InplaceUpdateTupleLock); + memset(repl_null, false, sizeof(repl_null)); memset(repl_repl, false, sizeof(repl_repl)); @@ -1807,6 +1816,7 @@ AlterDatabaseOwner(const char *dbname, Oid newOwnerId) newtuple = heap_modify_tuple(tuple, RelationGetDescr(rel), repl_val, repl_null, repl_repl); CatalogTupleUpdate(rel, &newtuple->t_self, newtuple); + UnlockTuple(rel, &tuple->t_self, InplaceUpdateTupleLock); heap_freetuple(newtuple); -- cgit v1.2.3