Speed up sessions list query #2934

macobo · 2021-01-13T14:42:40Z

Relevant issue: #2739

This PR rewrites how self-hosted postgres instances calculate sessions.

This query was slow because:

Combining events to sessions has quite a bit of business logic involved around previous events. The postgres function LAG was causing datasets to get re-sorted multiple times on postgres disk.
Because of the complicated business logic LIMIT could only be done at the very end -> much more data was processed than needed.
Events were loaded together with the session - slowing both calculations as well as 'data download' down for the users.

The solution is sort of silly:

instead of trying to do the business logic in postgres, stream rows to python and do the aggregation there. In practice, it seems like in this case the network io is less than doing all the extra work.
Only load events for LIMIT + offset users

One consequence of this is that pagination logic got quite bit more complex for sessions. We're now passing opaque data blobs back-and-forth from BE to FE and back again for this purpose.

Behavioral changes:

For self-hosted users, events are only loaded as they click to open the table.
Sessions are now sorted by end time

Checklist

All querysets/queries filter by Organization, by Team, and by User
Django backend tests
Jest frontend tests
Cypress end-to-end tests

Note clickhouse behavior is unchanged.

In addition to offset, postgres now returns a dict containing person_id, timestamp which is used to make sure we filter events on different pages correctly

Since we're ordering by end_time we know events before last end_time are all processed.

This gets used by the view

…view

macobo · 2021-01-13T14:48:46Z

Need to draft this for a bit - there are some bugs in here I'm finding.

EDsCODE

initial code comments. will manually QA in a bit

ee/clickhouse/views/events.py

EDsCODE · 2021-01-14T15:56:46Z

frontend/src/scenes/sessions/SessionDetails.tsx

    const [page, setPage] = useState(1)
    const [pageSize, setPageSize] = useState(50)

+    async function loadSessionEvents(): Promise<void> {


i think we're trying to keep most data logic in kea logics if possible. even if this data is scoped here only it'll be more modular if there's an accompanying logic instead of localstate

In this case the data was only used locally and the kea logic would have ended up quite a bit more convoluted.

But moving this out will become neccessary soon as we will want to show highlighted events in session recording. Ok if I punt on this until then?

yup just a heads up not strict

EDsCODE · 2021-01-14T15:58:35Z

posthog/api/event.py

+        return [session for i, session in enumerate(sessions) if session["distinct_id"] in person_ids]
+
+    @action(methods=["GET"], detail=False)
+    def session_events(self, request: request.Request, *args: Any, **kwargs: Any) -> Response:


does clickhouse also use this?

There's a bit of feature drift here.

For clickhouse, the sessions query still returns events and the frontend knows to use those. For postgres, events are loaded separately (for perf reasons) -> separate query is needed.

Not sure what the correct approach here is, I don't think it's worth refactoring clickhouse query here right now. Thoughts?

mm yeah I figured it was on these lines. I think if the frontend can handle then it's no problem. This will just be part of the consideration in consolidating our offering into clickhouse only. I think the duality will eventually be a major hinderance in moving quickly

EDsCODE

QA looks good!

macobo added 22 commits January 13, 2021 16:33

Remove (apparently) useless person joins from sessions sql

a5b1a8e

WIP: Make sessions list query use python iteration

5b68df1

WIP: Show loader while session data is loading

761fa08

Aggregate together sessions

3679149

Calculate start and end points of session separately

e9e72f1

Remove cruft code

5127aaa

Load session events asynchronously for self-hosted

12efd50

Note clickhouse behavior is unchanged.

Update pagination logic for sessions

8b87228

In addition to offset, postgres now returns a dict containing person_id, timestamp which is used to make sure we filter events on different pages correctly

Add some tests to SessionListBuilder

72811ea

Fix typing errors

7d41358

Fix pagination limit

4dcaaea

Move tests to right file

910dc94

Query less events for sessions list

d2e66f7

Since we're ordering by end_time we know events before last end_time are all processed.

Add test for current_url behavior

74862b6

Make sure old tests remain working

e37f5c6

Remove unused base class

42e1395

Move sessions-related queries to separate subfolder

67fb3e2

Extract sessions list code to separate file(s)

44de0cd

Sort sessions by end time in ch as well

fccedd4

List end time in sessions table

4798d61

Return person email when querying sessions lists on postgres

03fea00

This gets used by the view

Return email over all user properties for sessions in clickhouse and …

fb0fcc6

…view

macobo requested a review from EDsCODE January 13, 2021 14:42

timgl temporarily deployed to posthog-simplify-list-q-r6cq9h January 13, 2021 14:44 Inactive

macobo marked this pull request as draft January 13, 2021 14:48

macobo added 2 commits January 13, 2021 16:56

Fix an ordering bug

82966c2

Fix a pagination bug

156c043

macobo temporarily deployed to posthog-simplify-list-q-r6cq9h January 13, 2021 14:58 Inactive

Fix endpoint

7958f9f

macobo temporarily deployed to posthog-simplify-list-q-r6cq9h January 14, 2021 08:41 Inactive

macobo force-pushed the simplify-list-query branch from b9a8c4d to 880c153 Compare January 14, 2021 08:56

macobo temporarily deployed to posthog-simplify-list-q-r6cq9h January 14, 2021 08:56 Inactive

Fix basic sessions tests for pagination

8c215d8

macobo force-pushed the simplify-list-query branch from 880c153 to 8c215d8 Compare January 14, 2021 09:11

macobo temporarily deployed to posthog-simplify-list-q-r6cq9h January 14, 2021 09:11 Inactive

macobo temporarily deployed to posthog-simplify-list-q-r6cq9h January 14, 2021 11:21 Inactive

Sort consistently for sessions list builder

0ccf718

macobo force-pushed the simplify-list-query branch from 590f678 to 0ccf718 Compare January 14, 2021 11:24

macobo temporarily deployed to posthog-simplify-list-q-r6cq9h January 14, 2021 11:24 Inactive

macobo marked this pull request as ready for review January 14, 2021 12:30

macobo mentioned this pull request Jan 14, 2021

Support multiple action filters in sessions #2946

Merged

4 tasks

EDsCODE reviewed Jan 14, 2021

View reviewed changes

Roll pagination into filters

b3b6196

macobo temporarily deployed to posthog-simplify-list-q-r6cq9h January 14, 2021 20:16 Inactive

EDsCODE approved these changes Jan 14, 2021

View reviewed changes

macobo merged commit 17a31f0 into master Jan 14, 2021

macobo deleted the simplify-list-query branch January 14, 2021 23:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Speed up sessions list query #2934

Speed up sessions list query #2934

macobo commented Jan 13, 2021

macobo commented Jan 13, 2021

EDsCODE left a comment

EDsCODE Jan 14, 2021

macobo Jan 14, 2021

EDsCODE Jan 14, 2021

EDsCODE Jan 14, 2021

macobo Jan 14, 2021

EDsCODE Jan 14, 2021

EDsCODE left a comment

Speed up sessions list query #2934

Speed up sessions list query #2934

Conversation

macobo commented Jan 13, 2021

Checklist

macobo commented Jan 13, 2021

EDsCODE left a comment

Choose a reason for hiding this comment

EDsCODE Jan 14, 2021

Choose a reason for hiding this comment

macobo Jan 14, 2021

Choose a reason for hiding this comment

EDsCODE Jan 14, 2021

Choose a reason for hiding this comment

EDsCODE Jan 14, 2021

Choose a reason for hiding this comment

macobo Jan 14, 2021

Choose a reason for hiding this comment

EDsCODE Jan 14, 2021

Choose a reason for hiding this comment

EDsCODE left a comment

Choose a reason for hiding this comment