Add homepage panel stats to pings #14589
Merged
Conversation
This file contains 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
unknwon
reviewed
Oct 14, 2020
|
|
||
| func GetHomepagePanels(ctx context.Context) (*types.HomepagePanels, error) { | ||
| const q = ` | ||
| WITH sub AS (SELECT name, user_id FROM event_logs WHERE name LIKE '%Panel%' AND DATE_TRUNC('week', timestamp) = DATE_TRUNC('week', current_date)) |
There was a problem hiding this comment.
Full text search with '%Panel%' is much slower than a prefix-only search in my experience. Is it possible that we have a common prefix of all events for homepage panels?
For example, HomepagePanels::RecentFilesPanelFileClicked, in that case, we only need to do WHERE name LIKE 'HomepagePanels::%'. cc @tsenart
Here is the query plan I got for running this query in prod DB:
QUERY PLAN
--------------------------------------------------------------------------------------------------------------------------------------------------------------------
Aggregate (cost=590944.79..590944.90 rows=1 width=96) (actual time=1489.295..1489.296 rows=1 loops=1)
-> Gather (cost=1000.00..590942.26 rows=23 width=24) (actual time=1433.922..1715.514 rows=520 loops=1)
Workers Planned: 2
Workers Launched: 2
-> Parallel Seq Scan on event_logs (cost=0.00..589939.96 rows=10 width=24) (actual time=1431.128..1484.354 rows=173 loops=3)
Filter: ((name ~~ '%Panel%'::text) AND (date_trunc('week'::text, "timestamp") = date_trunc('week'::text, (CURRENT_DATE)::timestamp with time zone)))
Rows Removed by Filter: 3720315
Planning Time: 0.347 ms
Execution Time: 1716.090 ms
(9 rows)
There was a problem hiding this comment.
I took the liberty to improve the query and the method that runs it :) Please take a look. The query plan now looks like this:
Subquery Scan on sub (cost=2315.49..2315.62 rows=1 width=96) (actual time=24.003..24.003 rows=1 loops=1)
-> Aggregate (cost=2315.49..2315.58 rows=1 width=136) (actual time=24.000..24.000 rows=1 loops=1)
-> Bitmap Heap Scan on event_logs (cost=2126.86..2302.31 rows=155 width=24) (actual time=22.868..23.651 rows=545 loops=1)
Recheck Cond: ((name = ANY ('{RecentFilesPanelFileClicked,RecentFilesPanelLoaded,RecentSearchesPanelSearchClicked,RecentSearchesPanelLoaded,RepositoriesPanelRepoFilterClicked,RepositoriesPanelLoaded,SavedSearchesPanelSearchClicked,SavedSearchesPanelLoaded,SavedSearchesPanelCreateButtonClicked,RecentFilesPanelFileClicked,RecentFilesPanelLoaded}'::text[])) AND ("timestamp" >= date_trunc('week'::text, (CURRENT_DATE)::timestamp with time zone)))
Heap Blocks: exact=412
-> BitmapAnd (cost=2126.86..2126.86 rows=155 width=0) (actual time=22.792..22.792 rows=0 loops=1)
-> Bitmap Index Scan on event_logs_name (cost=0.00..139.34 rows=9692 width=0) (actual time=0.592..0.592 rows=3569 loops=1)
Index Cond: (name = ANY ('{RecentFilesPanelFileClicked,RecentFilesPanelLoaded,RecentSearchesPanelSearchClicked,RecentSearchesPanelLoaded,RepositoriesPanelRepoFilterClicked,RepositoriesPanelLoaded,SavedSearchesPanelSearchClicked,SavedSearchesPanelLoaded,SavedSearchesPanelCreateButtonClicked,RecentFilesPanelFileClicked,RecentFilesPanelLoaded}'::text[]))
-> Bitmap Index Scan on event_logs_timestamp (cost=0.00..1987.19 rows=183500 width=0) (actual time=21.979..21.979 rows=363041 loops=1)
Index Cond: ("timestamp" >= date_trunc('week'::text, (CURRENT_DATE)::timestamp with time zone))
Planning Time: 0.271 ms
Execution Time: 24.108 ms
Codecov Report
@@ Coverage Diff @@
## main #14589 +/- ##
==========================================
+ Coverage 52.13% 52.21% +0.07%
==========================================
Files 1558 1558
Lines 79322 79179 -143
Branches 7108 6948 -160
==========================================
- Hits 41356 41340 -16
+ Misses 34223 34102 -121
+ Partials 3743 3737 -6
|
unknwon
approved these changes
Oct 14, 2020
dadlerj
reviewed
Oct 14, 2020
| RecentRepositoriesClickedPercentage float64 | ||
| SavedSearchesClickedPercentage float64 | ||
| NewSavedSearchesClickedPercentage float64 | ||
| TotalPanelViews float64 |
There was a problem hiding this comment.
@ebrodymoore should this be a float64? What type is it in BigQuery? I assume an integer?
There was a problem hiding this comment.
I actually have them all as floats
There was a problem hiding this comment.
Sounds good. Not a big deal, but generally I'd recommend using the best matched number types where possible. E.g. for counts, there will never be fractional values, so ints are a better fit (no worries about weird rounding issues, they take less space, they help users make assumptions about the data that is being held here, etc., but again, not a big deal).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.