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

ifix: Try dedicated ThreadPoolExecutor for query_awaited #29232

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

timgl
Copy link
Collaborator

@timgl timgl commented Feb 26, 2025

Problem

For some reason the query doesn't start in production, but works fine locally. Might be something to do with asgi or whatever. Try this to see if it fixes it

Changes

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Does this work well for both Cloud and self-hosted?

How did you test this code?

@timgl timgl requested review from fuziontech and a team February 26, 2025 03:23
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Added a dedicated ThreadPoolExecutor for query processing in the query_awaited function to address production issues where queries weren't starting.

  • Created global QUERY_EXECUTOR with 50 max workers to handle concurrent query processing
  • Replaced asyncio.get_event_loop() with asyncio.get_running_loop() for better async context handling
  • Modified query_awaited to use the dedicated executor instead of the default one
  • Set thread naming prefix as "query_processor" for better debugging and monitoring
  • Added comment explaining the worker limit to ensure system isn't overwhelmed while allowing 200 simultaneous queries

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Contributor

@aspicer aspicer left a comment

Choose a reason for hiding this comment

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

🤷 no context

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants