Skip to content

Conversation

@cstyan
Copy link
Contributor

@cstyan cstyan commented Dec 1, 2025

This PR piggy backs on the agent API cached workspace added in #20662 to provide a fast path for avoiding GetWorkspaceByAgentID calls in dbauthz's GetWorkspaceAgentByID. This query is not the most expensive, but has a significant call volume at ~16 million calls per week.

Profiling shows that the majority of these calls are via the agent API's agentFn.
Screenshot 2025-12-01 at 2 37 21 PM

@cstyan cstyan changed the title perf: reduce calls to GetWorkspaceByID in GetWorkspaceAgentByID via cached workspace perf: reduce calls to GetWorkspaceByAgentID in GetWorkspaceAgentByID via cached workspace Dec 4, 2025
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a fast path test here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not very easy to do with the amount of mocking we do in some of these tests, especially given that tests circumvent actually calls to the DB to retrieve the agent in the agentFn

the function and test that are updated in dbauthz are the code path that will be executed in production when agentFn is called from any of the sub-APIs in the agentapi package

}
}

// Inject RBAC object into context for dbauthz fast path, avoid having to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any opportunity to create a helper for this logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

definitely! I'd just thought of that while implementing yet another PR for this same fastpath pattern, and added the helper function there.

I had planned to do all the cleanup to use the helper in existing places in a single PR after the existing PRs, easier for reviewers IMO, but I can refactor any one of the existing PRs to do the helper + reuse in there if you think that's easier.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I merged the change with the helper, and used tasks to have AI update the other call sites for context injection in the latest commit. LMK what you guys think @zedkipp @Emyrk , it essentially just makes the error handling more consistent, we really only reduced LOC by about 10?

Copy link
Member

@Emyrk Emyrk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The helper function to deduplicate code would be preferred 👍

@github-actions
Copy link

github-actions bot commented Dec 10, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@cstyan cstyan force-pushed the callum/agent-expensive-call branch from 8c4a0f5 to 067ab1d Compare December 10, 2025 00:21
@cstyan cstyan force-pushed the callum/agent-expensive-call branch from 4bc6a88 to 1ec456b Compare December 10, 2025 19:48
@cstyan cstyan changed the title perf: reduce calls to GetWorkspaceByAgentID in GetWorkspaceAgentByID via cached workspace perf: reduce calls to GetWorkspaceByAgentID in GetWorkspaceAgentByID Dec 10, 2025
@cstyan cstyan force-pushed the callum/agent-expensive-call branch from 1ec456b to 23187b4 Compare December 10, 2025 20:28
#21047)

This PR piggy backs on the agent API cached workspace added in earlier PRs to provide a fast path for avoiding `GetWorkspaceByID` calls in `GetLatestWorkspaceBuildByWorkspaceID` via injection of the workspaces RBAC object into the context. We can do this from the `agentConnectionMonitor` easily since we already cache the workspace.

---------

Signed-off-by: Callum Styan <callumstyan@gmail.com>
@cstyan cstyan force-pushed the callum/agent-expensive-call branch from 23187b4 to 3af884b Compare December 10, 2025 20:57
@cstyan cstyan merged commit 8ed1c1d into main Dec 10, 2025
30 checks passed
@cstyan cstyan deleted the callum/agent-expensive-call branch December 10, 2025 22:03
@github-actions github-actions bot locked and limited conversation to collaborators Dec 10, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants