Skip to content

Stream query results (LG-5351)#5618

Merged
zachmargolis merged 4 commits intomainfrom
margolis-stream-query-results
Nov 18, 2021
Merged

Stream query results (LG-5351)#5618
zachmargolis merged 4 commits intomainfrom
margolis-stream-query-results

Conversation

@zachmargolis
Copy link
Contributor

Problem: We're observing queries that take so long in prod that they get Postgres serialization errors (basically by the time the query finishes running, the results are too out of date with other updates)

Proposed solution has two parts

  1. Move some of the trickiest logic (the "new unique" logic) into Ruby
  2. Stream the results

My thinking is that one of the reasons the queries take so long is they have to write a lot of data out to temporary tables to calculate new uniques, so maybe if we move the calculation to Ruby memory, we can put less strain on the DB, and the queries themselves should be able to simply stream results instead of waiting for the entire thing to be ready

I tried this out in a Rails console in prod. Before about 50% of runs would get the PG serialization error, and with this, my 1/1 attempt was fine, no error

**Why**: The big UNION query takes so long it gets serialization errors
in prod, so this reworks it to do more logic in Ruby and stream results
iaa_end_date: iaa_range.end.to_s,
total_auth_count: auth_count,
unique_users: unique_users.count,
new_unique_users: new_unique_users.count,
Copy link
Contributor Author

@zachmargolis zachmargolis Nov 18, 2021

Choose a reason for hiding this comment

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

technically we do not currently need new_unique_users for the by-issuer report.... however my thinking is maybe it's just easier to leave in here? I checked and our upcming ticket LG-4975 does not need this either, so it's dead code

Happy to remove it also and put it back if we need it later

Copy link
Contributor

@stevegsa stevegsa left a comment

Choose a reason for hiding this comment

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

Excellent. Out of the box thinking....

@zachmargolis zachmargolis merged commit 9dec264 into main Nov 18, 2021
@zachmargolis zachmargolis deleted the margolis-stream-query-results branch November 18, 2021 18:48
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