Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix pprof handlers to not use a db transaction #2122

Closed
dnephin opened this issue May 27, 2022 · 4 comments
Closed

Fix pprof handlers to not use a db transaction #2122

dnephin opened this issue May 27, 2022 · 4 comments
Labels
area/api Issue or PR related to the Infra API. kind/bug A report or a fix for a problem with functional correctness. status/stale Used by actions/stale to mark an issue or PR as stale.

Comments

@dnephin
Copy link
Contributor

dnephin commented May 27, 2022

Our /debug/pprof handlers are not very useful right now because the middleware around them start a database transaction. We are currently limited to a single DB transaction at a time, so the profile running prevents any other requests from being handled.

In the short term we can address that by:

  1. making sure that the authn, authz, "update user lastSeenAt", and "update accessKey last used" operations use a separate transaction (or possibly no transaction). The transaction used by any middleware must be closed before they call the API handler
  2. API handlers that do require a transaction should start their own.
@dnephin dnephin added the kind/bug A report or a fix for a problem with functional correctness. label May 27, 2022
@jmorganca jmorganca added the area/api Issue or PR related to the Infra API. label May 30, 2022
@jmorganca jmorganca changed the title fix pprof handlers to not use a db transaction api: fix pprof handlers to not use a db transaction May 30, 2022
@jmorganca jmorganca changed the title api: fix pprof handlers to not use a db transaction Fix pprof handlers to not use a db transaction May 31, 2022
@github-actions
Copy link

This issue has not seen any activity in a while. Add a comment if this issue is still relevant,
otherwise it will be closed in 7 days.

@github-actions github-actions bot added the status/stale Used by actions/stale to mark an issue or PR as stale. label Jul 30, 2022
@dnephin dnephin removed the status/stale Used by actions/stale to mark an issue or PR as stale. label Aug 2, 2022
@dnephin
Copy link
Contributor Author

dnephin commented Aug 2, 2022

Still relevant

@stale
Copy link

stale bot commented Oct 1, 2022

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the status/stale Used by actions/stale to mark an issue or PR as stale. label Oct 1, 2022
@dnephin
Copy link
Contributor Author

dnephin commented Oct 2, 2022

Fixed by #3299, pprof handler now has a separate read-only txn.

@dnephin dnephin closed this as completed Oct 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api Issue or PR related to the Infra API. kind/bug A report or a fix for a problem with functional correctness. status/stale Used by actions/stale to mark an issue or PR as stale.
Projects
None yet
Development

No branches or pull requests

2 participants