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

Count results while printing them, when ignoring pagination #795

Merged
merged 11 commits into from
Oct 16, 2019

Conversation

mattmundell
Copy link
Contributor

@mattmundell mattmundell commented Oct 14, 2019

This skips some counting SQL by counting the results as they are being added
to the response XML.

This is possible because when we ignore_pagination we are getting all the
filtered results anyway, so we can do both the page and filtered counting as we
go through the results.

On my basic test report of 5000 results this saved about 212ms of a 1344ms
report, or 12%. To be clear: that's the time spent in SQL for the GET_REPORTS
call made by the GSA Report page.

Checklist:

@mattmundell mattmundell changed the title Count filtered results while printing them, when ignoring pagination Count results while printing them, when ignoring pagination Oct 15, 2019
@mattmundell mattmundell marked this pull request as ready for review October 15, 2019 10:59
src/manage_sql.c Outdated
Comment on lines 30138 to 30145
debugs = f_debugs;
holes = f_holes;
infos = f_infos;
logs = f_logs;
warnings = f_warnings;
false_positives = f_false_positives;
filtered_result_count = debugs + holes + infos + logs + warnings
+ false_positives;
Copy link
Member

Choose a reason for hiding this comment

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

Why are you overwriting the full counts with the filtered ones here instead of just adding the filtered counts for filtered_result_count?
Earlier you already get them with report_counts_id_full (...) and while the total is still correct after this the counts per level are not if there are any filter conditions limiting the results selection.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I misunderstood what the report_counts_id part was doing. Should be right with
299fefa. Thanks for catching. Really want to clean this code up, but need to backport these PRs to gvmd 8.

@@ -29483,6 +29483,8 @@ print_report_xml_start (report_t report, report_t delta, task_t task,
"</delta>");
}

count_filtered = (delta == 0 && ignore_pagination && get->details);
Copy link
Member

Choose a reason for hiding this comment

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

This could perhaps also be used in any case where the number of rows is unlimited via a rows=-1 in the filter and the "Max Rows" limit is disabled.
If "Max Rows" is enabled it could also be compared against the unfiltered total or some other upper bound that's quick to query to see if using the number of returned results could give the wrong count.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds like a good idea. Would like to leave it for a later PR though.

@timopollmeier timopollmeier merged commit c348af9 into greenbone:master Oct 16, 2019
@mattmundell mattmundell deleted the skip-count branch October 16, 2019 10:14
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