feat!(perf): coalesce identical queries and limit concurrent query execution#805
Open
shahzeelahmed wants to merge 30 commits intodevelopfrom
Open
feat!(perf): coalesce identical queries and limit concurrent query execution#805shahzeelahmed wants to merge 30 commits intodevelopfrom
shahzeelahmed wants to merge 30 commits intodevelopfrom
Conversation
shahzeelahmed
changed the title
shahzeelahmed
changed the title
| release_lock(lock_key) | ||
|
|
||
| def _publish_query_complete(self, lock_key, dashboard=None): | ||
| if not dashboard: |
Member
There was a problem hiding this comment.
The events should be sent regardless of dashboard param? Or is it intentionally meant only for dashboards?
IMO, results should be made available on the client via realtime events for query & chart builder as well, to avoid duplicate execution. In this case "dashboard" param is unnecessary (?)
| dashboard.refresh() | ||
|
|
||
| // avoid listening if user navigates to another dashboard room | ||
| onBeforeUnmount(() => { |
Member
There was a problem hiding this comment.
should be done in DashboardBuilder too?
| break | ||
| def _cache_execution_result(self, lock_key, result, cache_expiry=600): | ||
| cache_key = f"insights:exec_result:{lock_key}" | ||
| frappe.cache().set_value(cache_key, frappe.as_json(result), expires_in_sec=cache_expiry) |
Member
There was a problem hiding this comment.
can we eliminate this duplicate cache? results are already cached in execute_ibis_query function
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem: Each site has a limited number of worker threads to handle requests. when a dashboard has 20+ queries which takes more than a minute to execute, each thread gets blocked for the duration of I/O bound work. This causes the site to become slow and even downtime in some cases. now, the max number of queries which can run at a time will be capped at 6 by default and the client will poll for the retries.
query_hashfield in Insights Query v3TODO