Skip to content

Commit

Permalink
BABEL: Skip locking the tuple for ENR-related tuple modifications.
Browse files Browse the repository at this point in the history
Commit 09f0820095ea8b7c6ca4269454a94695c8421628 introduced
locking for individual tuples during certain operations. However,
Babelfish introduced the concept of ENR-only relations, which
store all catalog tuples in their own local cache and not in the
physical catalogs. As these relations and their catalog tuples
are completely session-local, there is no need to acquire locks on
tuples for these relations (and it would just lead to a crash
anyways since the tuples do not have an underlying TID to lock against).

Signed-off-by: Jason Teng <[email protected]>
(cherry picked from commit 60916a8f80f1164daade1fc707637ec93ff3ee68)
(cherry picked from commit e905197)
  • Loading branch information
Jason Teng authored and shardgupta committed Dec 11, 2024
1 parent d76e651 commit d4ab08a
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 21 deletions.
21 changes: 16 additions & 5 deletions src/backend/catalog/aclchk.c
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
#include "nodes/makefuncs.h"
#include "parser/parse_func.h"
#include "parser/parse_type.h"
#include "parser/parser.h"
#include "storage/lmgr.h"
#include "utils/acl.h"
#include "utils/aclchk_internal.h"
Expand Down Expand Up @@ -1823,8 +1824,12 @@ ExecGrant_Relation(InternalGrant *istmt)
Oid ownerId;
HeapTuple tuple;
ListCell *cell_colprivs;
bool is_enr = (sql_dialect == SQL_DIALECT_TSQL && get_ENR_withoid(currentQueryEnv, relOid, ENR_TSQL_TEMP));

tuple = SearchSysCacheLocked1(RELOID, ObjectIdGetDatum(relOid));
if (is_enr)
tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relOid));
else
tuple = SearchSysCacheLocked1(RELOID, ObjectIdGetDatum(relOid));
if (!HeapTupleIsValid(tuple))
elog(ERROR, "cache lookup failed for relation %u", relOid);
pg_class_tuple = (Form_pg_class) GETSTRUCT(tuple);
Expand Down Expand Up @@ -2040,7 +2045,8 @@ ExecGrant_Relation(InternalGrant *istmt)
values, nulls, replaces);

CatalogTupleUpdate(relation, &newtuple->t_self, newtuple);
UnlockTuple(relation, &tuple->t_self, InplaceUpdateTupleLock);
if (!is_enr)
UnlockTuple(relation, &tuple->t_self, InplaceUpdateTupleLock);

/* Update initial privileges for extensions */
recordExtensionInitPriv(relOid, RelationRelationId, 0, new_acl);
Expand All @@ -2053,7 +2059,7 @@ ExecGrant_Relation(InternalGrant *istmt)

pfree(new_acl);
}
else
else if (!is_enr)
UnlockTuple(relation, &tuple->t_self, InplaceUpdateTupleLock);

/*
Expand Down Expand Up @@ -2163,8 +2169,12 @@ ExecGrant_Database(InternalGrant *istmt)
Oid *oldmembers;
Oid *newmembers;
HeapTuple tuple;
bool is_enr = (sql_dialect == SQL_DIALECT_TSQL && get_ENR_withoid(currentQueryEnv, datId, ENR_TSQL_TEMP));

tuple = SearchSysCacheLocked1(DATABASEOID, ObjectIdGetDatum(datId));
if (is_enr)
tuple = SearchSysCache1(DATABASEOID, ObjectIdGetDatum(datId));
else
tuple = SearchSysCacheLocked1(DATABASEOID, ObjectIdGetDatum(datId));
if (!HeapTupleIsValid(tuple))
elog(ERROR, "cache lookup failed for database %u", datId);

Expand Down Expand Up @@ -2233,7 +2243,8 @@ ExecGrant_Database(InternalGrant *istmt)
nulls, replaces);

CatalogTupleUpdate(relation, &newtuple->t_self, newtuple);
UnlockTuple(relation, &tuple->t_self, InplaceUpdateTupleLock);
if (!is_enr)
UnlockTuple(relation, &tuple->t_self, InplaceUpdateTupleLock);

/* Update the shared dependency ACL info */
updateAclDependencies(DatabaseRelationId, pg_database_tuple->oid, 0,
Expand Down
11 changes: 9 additions & 2 deletions src/backend/commands/dbcommands.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
#include "common/file_perm.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
#include "parser/parser.h"
#include "pgstat.h"
#include "postmaster/bgwriter.h"
#include "replication/slot.h"
Expand Down Expand Up @@ -1755,6 +1756,7 @@ RenameDatabase(const char *oldname, const char *newname)
int notherbackends;
int npreparedxacts;
ObjectAddress address;
bool is_enr = false;

/*
* Look up the target database's OID, and get exclusive lock on it. We
Expand Down Expand Up @@ -1822,13 +1824,18 @@ RenameDatabase(const char *oldname, const char *newname)
errdetail_busy_db(notherbackends, npreparedxacts)));

/* rename */
newtup = SearchSysCacheLockedCopy1(DATABASEOID, ObjectIdGetDatum(db_id));
is_enr = (sql_dialect == SQL_DIALECT_TSQL && get_ENR_withoid(currentQueryEnv, db_id, ENR_TSQL_TEMP));
if (is_enr)
newtup = SearchSysCacheCopy1(DATABASEOID, ObjectIdGetDatum(db_id));
else
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, &otid, newtup);
UnlockTuple(rel, &otid, InplaceUpdateTupleLock);
if (!is_enr)
UnlockTuple(rel, &otid, InplaceUpdateTupleLock);

InvokeObjectPostAlterHook(DatabaseRelationId, db_id, 0);

Expand Down
11 changes: 9 additions & 2 deletions src/backend/commands/indexcmds.c
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
#include "parser/parse_coerce.h"
#include "parser/parse_func.h"
#include "parser/parse_oper.h"
#include "parser/parser.h"
#include "partitioning/partdesc.h"
#include "pgstat.h"
#include "rewrite/rewriteManip.h"
Expand All @@ -63,6 +64,7 @@
#include "utils/memutils.h"
#include "utils/partcache.h"
#include "utils/pg_rusage.h"
#include "utils/queryenvironment.h"
#include "utils/regproc.h"
#include "utils/snapmgr.h"
#include "utils/syscache.h"
Expand Down Expand Up @@ -4295,16 +4297,21 @@ update_relispartition(Oid relationId, bool newval)
HeapTuple tup;
Relation classRel;
ItemPointerData otid;
bool is_enr = (sql_dialect == SQL_DIALECT_TSQL && get_ENR_withoid(currentQueryEnv, relationId, ENR_TSQL_TEMP));

classRel = table_open(RelationRelationId, RowExclusiveLock);
tup = SearchSysCacheLockedCopy1(RELOID, ObjectIdGetDatum(relationId));
if (is_enr)
tup = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relationId));
else
tup = SearchSysCacheLockedCopy1(RELOID, ObjectIdGetDatum(relationId));
if (!HeapTupleIsValid(tup))
elog(ERROR, "cache lookup failed for relation %u", relationId);
otid = tup->t_self;
Assert(((Form_pg_class) GETSTRUCT(tup))->relispartition != newval);
((Form_pg_class) GETSTRUCT(tup))->relispartition = newval;
CatalogTupleUpdate(classRel, &otid, tup);
UnlockTuple(classRel, &otid, InplaceUpdateTupleLock);
if (!is_enr)
UnlockTuple(classRel, &otid, InplaceUpdateTupleLock);
heap_freetuple(tup);
table_close(classRel, RowExclusiveLock);
}
Expand Down
39 changes: 30 additions & 9 deletions src/backend/commands/tablecmds.c
Original file line number Diff line number Diff line change
Expand Up @@ -3438,13 +3438,17 @@ SetRelationTableSpace(Relation rel,
ItemPointerData otid;
Form_pg_class rd_rel;
Oid reloid = RelationGetRelid(rel);
bool is_enr = (sql_dialect == SQL_DIALECT_TSQL && get_ENR_withoid(currentQueryEnv, reloid, ENR_TSQL_TEMP));

Assert(CheckRelationTableSpaceMove(rel, newTableSpaceId));

/* Get a modifiable copy of the relation's pg_class row. */
pg_class = table_open(RelationRelationId, RowExclusiveLock);

tuple = SearchSysCacheLockedCopy1(RELOID, ObjectIdGetDatum(reloid));
if (is_enr)
tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));
else
tuple = SearchSysCacheLockedCopy1(RELOID, ObjectIdGetDatum(reloid));
if (!HeapTupleIsValid(tuple))
elog(ERROR, "cache lookup failed for relation %u", reloid);
otid = tuple->t_self;
Expand All @@ -3456,7 +3460,8 @@ SetRelationTableSpace(Relation rel,
if (OidIsValid(newRelFileNode))
rd_rel->relfilenode = newRelFileNode;
CatalogTupleUpdate(pg_class, &otid, tuple);
UnlockTuple(pg_class, &otid, InplaceUpdateTupleLock);
if (!is_enr)
UnlockTuple(pg_class, &otid, InplaceUpdateTupleLock);

/*
* Record dependency on tablespace. This is only required for relations
Expand Down Expand Up @@ -3954,6 +3959,7 @@ RenameRelationInternal(Oid myrelid, const char *newrelname, bool is_internal, bo
HeapTuple reltup;
Form_pg_class relform;
Oid namespaceId;
bool is_enr = (sql_dialect == SQL_DIALECT_TSQL && get_ENR_withoid(currentQueryEnv, myrelid, ENR_TSQL_TEMP));

/*
* Grab a lock on the target relation, which we will NOT release until end
Expand All @@ -3973,7 +3979,10 @@ RenameRelationInternal(Oid myrelid, const char *newrelname, bool is_internal, bo
*/
relrelation = table_open(RelationRelationId, RowExclusiveLock);

reltup = SearchSysCacheLockedCopy1(RELOID, ObjectIdGetDatum(myrelid));
if (is_enr)
reltup = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(myrelid));
else
reltup = SearchSysCacheLockedCopy1(RELOID, ObjectIdGetDatum(myrelid));
if (!HeapTupleIsValid(reltup)) /* shouldn't happen */
elog(ERROR, "cache lookup failed for relation %u", myrelid);
otid = reltup->t_self;
Expand Down Expand Up @@ -4002,7 +4011,8 @@ RenameRelationInternal(Oid myrelid, const char *newrelname, bool is_internal, bo
namestrcpy(&(relform->relname), newrelname);

CatalogTupleUpdate(relrelation, &otid, reltup);
UnlockTuple(relrelation, &otid, InplaceUpdateTupleLock);
if (!is_enr)
UnlockTuple(relrelation, &otid, InplaceUpdateTupleLock);

InvokeObjectPostAlterHookArg(RelationRelationId, myrelid, 0,
InvalidOid, is_internal);
Expand Down Expand Up @@ -14589,6 +14599,7 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
bool repl_null[Natts_pg_class];
bool repl_repl[Natts_pg_class];
static char *validnsps[] = HEAP_RELOPT_NAMESPACES;
bool is_enr = false;

if (defList == NIL && operation != AT_ReplaceRelOptions)
return; /* nothing to do */
Expand All @@ -14597,7 +14608,11 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,

/* Fetch heap tuple */
relid = RelationGetRelid(rel);
tuple = SearchSysCacheLocked1(RELOID, ObjectIdGetDatum(relid));
is_enr = (sql_dialect == SQL_DIALECT_TSQL && get_ENR_withoid(currentQueryEnv, relid, ENR_TSQL_TEMP));
if (is_enr)
tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
else
tuple = SearchSysCacheLocked1(RELOID, ObjectIdGetDatum(relid));
if (!HeapTupleIsValid(tuple))
elog(ERROR, "cache lookup failed for relation %u", relid);

Expand Down Expand Up @@ -14701,7 +14716,8 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
repl_val, repl_null, repl_repl);

CatalogTupleUpdate(pgclass, &newtuple->t_self, newtuple);
UnlockTuple(pgclass, &tuple->t_self, InplaceUpdateTupleLock);
if (!is_enr)
UnlockTuple(pgclass, &tuple->t_self, InplaceUpdateTupleLock);

InvokeObjectPostAlterHook(RelationRelationId, RelationGetRelid(rel), 0);

Expand Down Expand Up @@ -16897,9 +16913,13 @@ AlterRelationNamespaceInternal(Relation classRel, Oid relOid,
Form_pg_class classForm;
ObjectAddress thisobj;
bool already_done = false;
bool is_enr = (sql_dialect == SQL_DIALECT_TSQL && get_ENR_withoid(currentQueryEnv, relOid, ENR_TSQL_TEMP));

/* no rel lock for relkind=c so use LOCKTAG_TUPLE */
classTup = SearchSysCacheLockedCopy1(RELOID, ObjectIdGetDatum(relOid));
if (is_enr)
classTup = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relOid));
else
classTup = SearchSysCacheLockedCopy1(RELOID, ObjectIdGetDatum(relOid));
if (!HeapTupleIsValid(classTup))
elog(ERROR, "cache lookup failed for relation %u", relOid);
classForm = (Form_pg_class) GETSTRUCT(classTup);
Expand Down Expand Up @@ -16933,7 +16953,8 @@ AlterRelationNamespaceInternal(Relation classRel, Oid relOid,
classForm->relnamespace = newNspOid;

CatalogTupleUpdate(classRel, &otid, classTup);
UnlockTuple(classRel, &otid, InplaceUpdateTupleLock);
if (!is_enr)
UnlockTuple(classRel, &otid, InplaceUpdateTupleLock);


/* Update dependency on schema if caller said so */
Expand All @@ -16946,7 +16967,7 @@ AlterRelationNamespaceInternal(Relation classRel, Oid relOid,
elog(ERROR, "failed to change schema dependency for relation \"%s\"",
NameStr(classForm->relname));
}
else
else if (!is_enr)
UnlockTuple(classRel, &classTup->t_self, InplaceUpdateTupleLock);
if (!already_done)
{
Expand Down
15 changes: 12 additions & 3 deletions src/backend/utils/cache/relcache.c
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
#include "optimizer/optimizer.h"
#include "parser/parser.h"
#include "postmaster/autovacuum.h"
#include "pgstat.h"
#include "rewrite/rewriteDefine.h"
#include "rewrite/rowsecurity.h"
Expand Down Expand Up @@ -3731,6 +3733,7 @@ RelationSetNewRelfilenode(Relation relation, char persistence)
MultiXactId minmulti = InvalidMultiXactId;
TransactionId freezeXid = InvalidTransactionId;
RelFileNode newrnode;
bool is_enr = false;

if (!IsBinaryUpgrade)
{
Expand Down Expand Up @@ -3768,8 +3771,13 @@ RelationSetNewRelfilenode(Relation relation, char persistence)
*/
pg_class = table_open(RelationRelationId, RowExclusiveLock);

tuple = SearchSysCacheLockedCopy1(RELOID,
ObjectIdGetDatum(RelationGetRelid(relation)));
is_enr = (sql_dialect == SQL_DIALECT_TSQL && get_ENR_withoid(currentQueryEnv, RelationGetRelid(relation), ENR_TSQL_TEMP));
if (is_enr)
tuple = SearchSysCacheCopy1(RELOID,
ObjectIdGetDatum(RelationGetRelid(relation)));
else
tuple = SearchSysCacheLockedCopy1(RELOID,
ObjectIdGetDatum(RelationGetRelid(relation)));
if (!HeapTupleIsValid(tuple))
elog(ERROR, "could not find tuple for relation %u",
RelationGetRelid(relation));
Expand Down Expand Up @@ -3896,7 +3904,8 @@ RelationSetNewRelfilenode(Relation relation, char persistence)
CatalogTupleUpdate(pg_class, &otid, tuple);
}

UnlockTuple(pg_class, &otid, InplaceUpdateTupleLock);
if (!is_enr)
UnlockTuple(pg_class, &otid, InplaceUpdateTupleLock);
heap_freetuple(tuple);

table_close(pg_class, RowExclusiveLock);
Expand Down
9 changes: 9 additions & 0 deletions src/backend/utils/cache/syscache.c
Original file line number Diff line number Diff line change
Expand Up @@ -1261,6 +1261,12 @@ ReleaseSysCache(HeapTuple tuple)
*
* The returned tuple may be the subject of an uncommitted update, so this
* doesn't prevent the "tuple concurrently updated" error.
*
* Note: For Babelfish, this function should not be used if the target tuple is
* for an ENR entry, as there is no physical tid for ENR catalog tuples (since ENR
* entries hold all catalog data internally in their cache). Use SearchSysCache1() instead
* to look up tuples for catalogs for ENR entries, and also skip the UnlockTuple() call
* in such cases.
*/
HeapTuple
SearchSysCacheLocked1(int cacheId,
Expand Down Expand Up @@ -1374,6 +1380,9 @@ SearchSysCacheCopy(int cacheId,
* Meld SearchSysCacheLockedCopy1 with SearchSysCacheCopy(). After the
* caller's heap_update(), it should UnlockTuple(InplaceUpdateTupleLock) and
* heap_freetuple().
*
* See SearchSysCacheLocked1() for notes about ENR entries (do not use this
* function for tuples related to ENR entries).
*/
HeapTuple
SearchSysCacheLockedCopy1(int cacheId,
Expand Down

0 comments on commit d4ab08a

Please sign in to comment.