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(init): uWSGI Snuba API processes not initializing Snuba #3370

Merged

Conversation

rahul-kumar-saini
Copy link
Contributor

Overview

  • When uWSGI starts a new Snuba API process, it simply starts the Flask app
  • This is a problem as the first query which hits this new process will force Snuba to load all datasets/entities/storages from config before the query can be executed
  • This is causing timeouts as queries which normally take <1s will randomly take >5s to execute

Before State

  • First query to any Snuba API process loads all datasets

After State

  • Datasets are loaded before the Snuba API is declared "ready" by uWSGI

Blast Radius

  • Snuba API
  • Initialization is more explicit once again

Testing Notes

  • Tested locally to verify the old version forces datasets to load on first query and now it does not 👨‍🔬

@rahul-kumar-saini rahul-kumar-saini requested a review from a team as a code owner November 10, 2022 23:11
@@ -29,8 +29,8 @@ def _load_entities() -> None:
initialize_entity_factory()


def initialize() -> None:
logger.info("Initializing snuba")
def initialize_snuba() -> None:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simple rename to make it easier to understand the usage of this function outside of this module.

from snuba.environment import setup_logging, setup_sentry

setup_logging()
setup_sentry()
initialize_snuba()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the core change, the snuba/web/wsgi.py file is loaded by uWSGI to start the API process.

Comment on lines 4 to +6
setup_logging()
setup_sentry()
initialize_snuba()
Copy link
Member

Choose a reason for hiding this comment

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

not related to this ticket but I'm starting to think all of these should be in the initialize function

@codecov-commenter
Copy link

codecov-commenter commented Nov 10, 2022

Codecov Report

Base: 90.76% // Head: 91.33% // Increases project coverage by +0.56% 🎉

Coverage data is based on head (589aa77) compared to base (8f23e9f).
Patch coverage: 71.42% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3370      +/-   ##
==========================================
+ Coverage   90.76%   91.33%   +0.56%     
==========================================
  Files         703      705       +2     
  Lines       32403    32541     +138     
==========================================
+ Hits        29410    29720     +310     
+ Misses       2993     2821     -172     
Impacted Files Coverage Δ
snuba/web/wsgi.py 0.00% <0.00%> (ø)
snuba/cli/__init__.py 86.36% <100.00%> (+18.18%) ⬆️
snuba/core/initialize.py 100.00% <100.00%> (ø)
test_initialization/test_initialize.py 100.00% <100.00%> (ø)
snuba/state/__init__.py 71.56% <0.00%> (-0.48%) ⬇️
snuba/admin/clickhouse/querylog.py 100.00% <0.00%> (ø)
snuba/admin/migrations_policies.py 100.00% <0.00%> (ø)
test_distributed_migrations/conftest.py 100.00% <0.00%> (ø)
...est_distributed_migrations/test_runner_add_node.py 100.00% <0.00%> (ø)
tests/datasets/test_replays_processor.py 99.56% <0.00%> (+0.20%) ⬆️
... and 9 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@rahul-kumar-saini rahul-kumar-saini enabled auto-merge (squash) November 10, 2022 23:20
@rahul-kumar-saini rahul-kumar-saini merged commit d1e5c26 into master Nov 10, 2022
@rahul-kumar-saini rahul-kumar-saini deleted the rahul/fix/explicitly-initialize-snuba-uwsgi branch November 10, 2022 23:30
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