Skip to content

Commit

Permalink
Fixes visibility problems with dependency propagation (#7028)
Browse files Browse the repository at this point in the history
**Problem:**
Previously we always used an outside superuser connection to overcome
permission issues for the current user while propagating dependencies.
That has mainly 2 problems:
1. Visibility issues during dependency propagation, (metadata connection
propagates some objects like a schema, and outside transaction does not
see it and tries to create it again)
2. Security issues (it is preferrable to use current user's connection
instead of extension superuser)

**Solution (high level):**
Now, we try to make a smarter decision on whether should we use an
outside superuser connection or current user's metadata connection. We
prefer using current user's connection if any of the objects, which is
already propagated in the current transaction, is a dependency for a
target object. We do that since we assume if current user has
permissions to create the dependency, then it can most probably
propagate the target as well.

Our assumption is expected to hold most of the times but it can still be
wrong. In those cases, transaction would fail and user should set the
GUC `citus.create_object_propagation` to `deferred` to work around it.

**Solution:**
1. We track all objects propagated in the current transaction (we can
handle subtransactions),
2. We propagate dependencies via the current user's metadata connection
if any dependency is created in the current transaction to address
issues listed above. Otherwise, we still use an outside superuser
connection.


DESCRIPTION: Fixes some object propagation errors seen with transaction
blocks.

Fixes #6614

---------

Co-authored-by: Nils Dijk <nils@citusdata.com>
  • Loading branch information
aykut-bozkurt and thanodnl committed Sep 5, 2023
1 parent 9f06773 commit 8eb3360
Show file tree
Hide file tree
Showing 12 changed files with 813 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1659,6 +1659,7 @@ PropagatePrerequisiteObjectsForDistributedTable(Oid relationId)
ObjectAddress *tableAddress = palloc0(sizeof(ObjectAddress));
ObjectAddressSet(*tableAddress, RelationRelationId, relationId);
EnsureAllObjectDependenciesExistOnAllNodes(list_make1(tableAddress));
TrackPropagatedTableAndSequences(relationId);
}


Expand Down
34 changes: 27 additions & 7 deletions src/backend/distributed/commands/dependencies.c
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,35 @@ EnsureDependenciesExistOnAllNodes(const ObjectAddress *target)
dependency->objectSubId, ExclusiveLock);
}

WorkerNode *workerNode = NULL;
foreach_ptr(workerNode, workerNodeList)

/*
* We need to propagate dependencies via the current user's metadata connection if
* any dependency for the target is created in the current transaction. Our assumption
* is that if we rely on a dependency created in the current transaction, then the
* current user, most probably, has permissions to create the target object as well.
* Note that, user still may not be able to create the target due to no permissions
* for any of its dependencies. But this is ok since it should be rare.
*
* If we opted to use a separate superuser connection for the target, then we would
* have visibility issues since propagated dependencies would be invisible to
* the separate connection until we locally commit.
*/
if (HasAnyDependencyInPropagatedObjects(target))
{
const char *nodeName = workerNode->workerName;
uint32 nodePort = workerNode->workerPort;
SendCommandListToWorkersWithMetadata(ddlCommands);
}
else
{
WorkerNode *workerNode = NULL;
foreach_ptr(workerNode, workerNodeList)
{
const char *nodeName = workerNode->workerName;
uint32 nodePort = workerNode->workerPort;

SendCommandListToWorkerOutsideTransaction(nodeName, nodePort,
CitusExtensionOwnerName(),
ddlCommands);
SendCommandListToWorkerOutsideTransaction(nodeName, nodePort,
CitusExtensionOwnerName(),
ddlCommands);
}
}

/*
Expand Down
1 change: 1 addition & 0 deletions src/backend/distributed/commands/utility_hook.c
Original file line number Diff line number Diff line change
Expand Up @@ -937,6 +937,7 @@ ProcessUtilityInternal(PlannedStmt *pstmt,
foreach_ptr(address, addresses)
{
MarkObjectDistributed(address);
TrackPropagatedObject(address);
}
}
}
Expand Down

0 comments on commit 8eb3360

Please sign in to comment.