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

Optimizations for missing one implementation #942

Merged
merged 10 commits into from
Dec 4, 2024
Merged

Conversation

jcscottiii
Copy link
Collaborator

@jcscottiii jcscottiii commented Nov 27, 2024

Background

Queries to the new /v1/stats/features/browsers/{browser}/missing_one_implementation_counts take too long. (~53 seconds just in the database layer)

Upon looking at the Query Insights for the endpoint, we can see that it has a very high latency and high number of rows scanned.

image

This change aims to fix these problems.

Summary of changes

  1. Modified the existing query template
  2. Add new indexes: BrowserFeatureSupportEvents_FeatureStatusOnBrowserRelease and BrowserFeatureSupportEvents_AllFeatureStatusesOnBrowserRelease
  3. Create a separate query for local environments
  4. Modified the fake data script

Change 1: Modified the existing query template

These are the changes in the variable: gcpMissingOneImplCountRawTemplate

Removed join with web features table

Before:
image

Now:
We actually do not need to join with the Web Features table for the IDs since BrowserFeatureSupportEvents has a foreign key on the WebFeatures.

Do better joins

  1. Moved to using INNER JOINs instead of LEFT JOINS. This removed the need to check if something was null
  2. The loop on template variable OtherBrowsersParamNames uses bfse.EventReleaseDate instead of releases.EventReleaseDate
  3. (Non performance related): I saw that we have future release dates stored (example). I limited the release dates to only focus on those < CURRENT_TIMESTAMP()

Change 2: Add new indexes: BrowserFeatureSupportEvents_FeatureStatusOnBrowserRelease and BrowserFeatureSupportEvents_AllFeatureStatusesOnBrowserRelease

These indexes help with quicker matching. More comments on the indexes themselves

Change 3: Create a separate query for local environments

This is similar to what we do for features search [1]. The gcp query is super fast in GCP but takes more than 1 minute in the emulator. And the only way to solve it is to add some CTEs. If we try the same CTE query used locally in GCP, it takes forever too:

image

As a result, we will maintain two versions. I also modified the tests to assert that both versions work.

Change 4: Modified the fake data script

The precalcuation step for the BrowserFeatureSupportEvents table was not being called. Now it is. And now, we can get data. It only looks at a year of release data to limit the data loaded into the database.

Also, I added new logic in a method called: generateMissingOneImplementations. This method actually makes it so that we get nice graph-able results. useful for the screenshots in #838

As a result of generateMissingOneImplementations, many features were marked as implemented across all Browsers. I need to add generateUnimplementedFeatures to match the existing functionality where we show a red X. (There were some features showing the red X but not as many. So I would rather force more features to show the red X in a separate method instead of making the implementation to generateMissingOneImplementations too complicated.

Summary

Now the call takes less than 2 seconds

image

@jcscottiii jcscottiii force-pushed the jcscottiii/missing_one branch from ca72c33 to 4e57e95 Compare November 27, 2024 14:30
@jcscottiii jcscottiii force-pushed the jcscottiii/missing_one branch from 7da3927 to d6da6dd Compare November 27, 2024 18:15
@KyleJu
Copy link
Collaborator

KyleJu commented Dec 4, 2024

Thanks for the detailed explanations!

@jcscottiii jcscottiii added this pull request to the merge queue Dec 4, 2024
Merged via the queue into main with commit 5d7739b Dec 4, 2024
6 checks passed
@jcscottiii jcscottiii deleted the jcscottiii/missing_one branch December 4, 2024 20:40
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