Page MenuHomePhabricator

Improve performance of instance visibility policy checks for administrative/deployment agents
Closed, ResolvedPublic

Description

Phacility instances do a policy check for InstancesManageCapability::CAPABILITY when verifying a viewer can examine an instance.

Administrators and deployment agents reach this check and query it serially if it's a custom policy. This negatively impacts performance of administrative operations ("list all instances") and deployment operations ("get all instances on this host").

For example, when querying instances on a host, we fetch and evaluate the same policy hundreds of times. Querying and deployment could be faster by fixing this.

Revisions and Commits

Restricted Differential Revision
Restricted Differential Revision

Event Timeline

One possible approach is to cache policy checks (when we've determined that user X can see object Y, cache that for the remainder of the request), but I don't want to do this idly since it has far-reaching implications, and there are cases where this cache could potentially produce the wrong result (for example, when we predict the effects of making a policy change during an edit, we evaluate objects as though they had a different policy than they really have).

Another possible approach is to express this as an extended policy instead of a policy exception. This may be straightforward.

Another possible approach is to grant instance managers an authority earlier, but that feels pretty hacky.

Using an extended policy sort of fixes this, except that extended policies currently only strengthen policies, so I also have to weaken the default View policy. This potentially makes the UI misleading, since it suggests that "all users" can view an instance, which isn't true.

Maybe I'll just put some sort of authority grant cache on this for now since that's very simple.

epriestley added a revision: Restricted Differential Revision.Jul 9 2016, 1:28 PM
epriestley closed this task as Resolved by committing Restricted Diffusion Commit.Jul 10 2016, 3:04 PM
epriestley added a commit: Restricted Diffusion Commit.
epriestley added a commit: Restricted Diffusion Commit.Sep 3 2016, 1:10 AM