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 view runtime for controllers with async queries #50634

Merged
merged 1 commit into from Jan 7, 2024

Conversation

fatkodima
Copy link
Member

Currently, when there are async queries inside the controller, we get a negative view runtime reported. Something like

Completed 200 OK in 1151ms (Views: -500.2ms | ActiveRecord: 1088.3ms | Allocations: 18532)

To reproduce, add something like this to the controller's action:

@users = User.select("pg_sleep(1)").load_async
sleep(0.5)

That is because when a query is executed in a separate thread, we increment the db runtime for the query

ActiveSupport::Notifications.monotonic_subscribe("sql.active_record") do |name, start, finish, id, payload|
ActiveRecord::RuntimeRegistry.sql_runtime += (finish - start)
end
Even though the time spent inside the view for the query is less than that.
This way, we get a db execution time larger than the view rendering time (even though that queries are executed as part of the view rendering) and when we subtract them to get just the view rendering time here we get a negative value.

In reality, we should subtract the time spent for queries inside the main thread, not the time spent executing the query inside the database.

cc @byroot (as implementor of async features)

@byroot byroot merged commit 7060d68 into rails:main Jan 7, 2024
4 checks passed
byroot added a commit that referenced this pull request Jan 7, 2024
…queries

Fix view runtime for controllers with async queries
@fatkodima fatkodima deleted the fix-view-runtime-with-async-queries branch January 7, 2024 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants