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

✨ Better perf using the cursor. #416

Merged
merged 18 commits into from
Jun 22, 2023
Merged

✨ Better perf using the cursor. #416

merged 18 commits into from
Jun 22, 2023

Conversation

jortel
Copy link
Contributor

@jortel jortel commented Jun 21, 2023

To support pagination, the hub sets the X-Total header to tell the caller (The UI really) the total number available to be displayed as the total number of pages. To accomplish this, the hub executes the query 2 times. First to get the count(*), again to fetch the data. This doubles the latency and overhead in the hub for each request. Unfortunately, the sq.Rows does not provide the count.

Better performance iterating the cursor (sql.Rows) instead of running the query twice. This also eliminates the race condition between the count (query) and fetch query.

Adds a new cursor object used to consistently:

  • execute the query
  • iterate the sql.Rows
  • scan
  • paginate
  • count

The count is capped at 50,000 because for tables (like incident) the total can be tens-hundreds of millions. Iterating the rows to determine the count becomes expensive (cpu/time) with those numbers. The X-Total header is only used to support/improve pagination. After a certain point, extremely large numbers become meaningless since users will never page though them. After a discussion with @mturley, the hub will set X-Total with a leading > when capped. Example: X-Total: >50000. The UI will display accordingly.

50,000 may/may-not be the appropriate number.

PoC using the largest table Incident and one of the reports: RuleReport.

@jortel jortel marked this pull request as ready for review June 21, 2023 16:56
@jortel jortel requested a review from mansam June 21, 2023 19:09
Copy link
Collaborator

@mansam mansam left a comment

Choose a reason for hiding this comment

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

LGTM

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