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

chore(p-s): add logs around setupPlugins #18099

Merged
merged 1 commit into from
Oct 19, 2023
Merged

chore(p-s): add logs around setupPlugins #18099

merged 1 commit into from
Oct 19, 2023

Conversation

xvello
Copy link
Contributor

@xvello xvello commented Oct 19, 2023

Problem

Let's stop blaming mmdb, this is the real reason for prod-us pods to be so slow to start.

Changes

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

How did you test this code?

@xvello xvello requested a review from a team October 19, 2023 15:30
@xvello xvello merged commit a02e816 into master Oct 19, 2023
69 checks passed
@xvello xvello deleted the xvello/pclog branch October 19, 2023 15:57
@tiina303
Copy link
Contributor

tiina303 commented Oct 23, 2023

We likely don't need to load all of the plugins, just ones with processEvent <- do we load all of them? (as in we could do better filtering 🤷‍♀️ )

@xvello
Copy link
Contributor Author

xvello commented Oct 24, 2023

@tiina303 we already instantiate only the relevant VMs thanks to shouldSetupPluginInServer (based on the plugin's exported functions and the pod's capabilities), but we still pull everything from PG.

Instantiating the VM is the really costly part, so optimizing the PG query might not be the highest impact. We could indeed filter the PG query based on the capabilities column, but we'd need to make sure this colum is updated out of band when a plugin is installed/updated. Right now, it is updated by setupPlugins, so this function needs to load everything :/

Justicea83 pushed a commit to Justicea83/posthog that referenced this pull request Oct 25, 2023
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.

3 participants